Skip to content
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

Switch persistent worker from Protocol Buffers to JSON #133

Merged
merged 6 commits into from
Dec 19, 2022

Conversation

artem-zinnatullin
Copy link
Contributor

@artem-zinnatullin artem-zinnatullin commented Dec 16, 2022

Based on @arturdryomov work in #107 rebased w/ current master, updated to Bazel changes and latest deps.

Switches Detekt Persistent Worker to JSON, thus allowing us to remove dependency on rules_proto and speedup dep resolution for downstream users of rules_detekt!

Back then when @arturdryomov made original PR Bazel still had JSON as experimental option which since was set to true by default and is now a stable feature w/o feature-flag, and there was a bug with newline delimeters that is now fixed.

This means that users on stable new versions of Bazel (since 2022-01-24) don't have to do anything on their end to use rules_detekt with this change and they'll just get speedup because of removed dependency on rules_proto.

@Bencodes
Copy link
Contributor

We probably need to make a note in the README that you need to enable json worker support if you are using an older version of Bazel.

Double check, but I think the json worker protocol flag only got flipped to true recently in 5.x or 6.x. Bazel 4.x still has another year before it's no longer supported.

https://blog.bazel.build/2020/11/10/long-term-support-release.html

@artem-zinnatullin
Copy link
Contributor Author

Added info on the --experimental_worker_allow_json_protocol to the README!

@artem-zinnatullin artem-zinnatullin merged commit 795d785 into master Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants