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

chore: update to latest rules_nodejs #1207

Merged
merged 1 commit into from
Oct 10, 2023
Merged

Conversation

alexeagle
Copy link
Contributor

Simplify by writing the buildozer program in JavaScript rather than TypeScript so we don't need any devDependencies

@vladmos
Copy link
Member

vladmos commented Oct 10, 2023

There are build failures for the PR, could you please take a look at them?

@alexeagle alexeagle force-pushed the master branch 2 times, most recently from 32c09b9 to bc087d7 Compare October 10, 2023 14:10
@alexeagle
Copy link
Contributor Author

Green now. I also added an optional pre-commit setup so CI doesn't yell at me about buildifier. (It doesn't even print which files had format/lint warnings...)

Simplify by writing the buildozer program in JavaScript rather than TypeScript so we don't need any devDependencies
@vladmos
Copy link
Member

vladmos commented Oct 10, 2023

Thanks!

Do I understand correctly that the precommit hook only those who explicitly installed pre-commit on their development machines?

@alexeagle
Copy link
Contributor Author

alexeagle commented Oct 10, 2023

Correct, all git commit hook libraries have reached consensus that developers should opt-in. No magic going on.

The only thing maybe worth improving here is for a failing CI run to recommend the developer run pre-commit install in their clone of the repo to avoid having more bad round-trips through CI.

@vladmos vladmos merged commit eb5f67e into bazelbuild:master Oct 10, 2023
1 check passed
apattidb pushed a commit to databricks/buildtools that referenced this pull request May 10, 2024
Simplify by writing the buildozer program in JavaScript rather than TypeScript so we don't need any devDependencies
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.

2 participants