-
-
Notifications
You must be signed in to change notification settings - Fork 50
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
feat: merge bazel-super-formatter repo here #14
Conversation
Follow this approach where the tools must be declared in userland
Was this decided on or just a POC? Was there a discussion somewhere? |
The idea came from a PR on the super-formatter to add bzlmod. Then it just magically fell together in a couple hours. Let's discuss 🥹 |
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.
LGTM source-wise, I can help with migrating remaining tools. One of my interns wanted to add rustfmt as well.
I'll test how it works in practice on Monday.
What about rules_jvm_external and scalafmt?
And what do you think about my idea of testing bzlmod/workspace correspondence?
Adding rules_jvm_external here is fine since it goes in the users project. I'm sorry you went to all the trouble to vendor individual jar files in the other project! |
My idea is to support only bzlmod here and only workspace in bazel-super-formatter.. can you say more about your idea for correspondence? |
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.
🦖
Follow this approach where the tools must be declared in userland
Potentially replaces aspect-build/bazel-super-formatter#49 and results in us archiving bazel-super-formatter :(
/cc @jsharpe @agluszak