-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create blert v0.2.0 #6039
base: master
Are you sure you want to change the base?
Create blert v0.2.0 #6039
Conversation
Includes non-plugin changes New plugin |
This is a pretty large and complex project, so I'd be happy to discuss it in more detail as it is reviewed. |
This should be ready to review now. Happy to chat about anything, and please reach out before merging. |
The chances this gets reviewed in a timely manor is very low based on the number of dependencies added |
No worries, I don't expect a quick review. Was just pointing out that I don't intend to make any further commits. |
thirdParty("com.google.gradle:osdetector-gradle-plugin:1.7.3") { | ||
because "blert" | ||
} | ||
thirdParty("kr.motd.maven:os-maven-plugin:1.7.1") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this used for? Both your plugin and the plugin hub are gradle projects, not maven projects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried looking into it but couldn't find exactly where this is pulled in -- I assume it's a transitive of one of the protobuf libraries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I was looking the in wrong place. It is indeed coming from protobuf:
+--- com.google.protobuf:protobuf-gradle-plugin:0.9.4
| \--- com.google.gradle:osdetector-gradle-plugin:1.7.3
| \--- kr.motd.maven:os-maven-plugin:1.7.1
| \--- com.google.code.findbugs:jsr305:3.0.2
This would make the build non-reproduceable, please don't include it. |
Removed the remote check, it wasn't actually being used. |
On the dependency issue: The plugin pulls in a single direct dependency (the protobuf compiler), which is quite large and has a bunch of transitives. However, this dependency is only required at build time to generate Java code from the plugin's It is possible to remove the protoc dependency by manually running the compiler and checking in the generated Java protobuf code. This currently amounts to ~14.5K LOC (26K lines total) for serializing/deserializing protobuf messages to Java objects. Does Runelite have a policy around checking in/reviewing generated code of this nature, and would it be preferable to eliminate the protoc dependency by doing so? |
Blert is a PvM tracking and data analysis system hosted at https://blert.io.
This plugin collects data about players' raids and transmits it to the Blert servers to be processed and visualized.