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.

Sign up for GitHub

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

Jump to bottom

Introduce pre push hook for Bazel #2107

Closed
anandwana001 opened this issue Nov 13, 2020 · 2 comments
Closed

Introduce pre push hook for Bazel #2107

anandwana001 opened this issue Nov 13, 2020 · 2 comments
Labels
enhancement End user-perceivable enhancements. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Work: High It's not clear what the solution is. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@anandwana001
Copy link
Contributor

anandwana001 commented Nov 13, 2020 •
edited
Loading

Is your feature request related to a problem? Please describe.
As we are migrating towards the bazel, we have a bunch of bazel files coming on our way, we are adding a linter to github action at pr #2048 so we also require to prevent any error before pushing.

Describe the solution you'd like
Introducing pre-push will prevent nit errors and contributors can fix before pushing.

Pre-push hook is introducing in #2002 pr.

Tried few steps to introduce pre-push in the PR #2048

Things that don't worked out:

  1. Local buildifier file is not working correctly
  2. Need to test on windows - Fix #1736: Add Bazel Linter #2048 (comment)

If you would like to work on this issue, please reach out to @anandwana001
This was referenced Nov 13, 2020
Merged
Closed
@BenHenning
Copy link
Member

We should also add a script to perform the linting check (including reliably finding all files to run through buildifier).

@anandwana001 anandwana001 changed the title Introduce pre push hook for Bazel [Blocked on #2002] Introduce pre push hook for Bazel Jan 5, 2021
@anandwana001 anandwana001 self-assigned this Jan 6, 2021
@anandwana001 anandwana001 removed their assignment Feb 4, 2021
@Broppia Broppia added dev_team Impact: Low Low perceived user impact (e.g. edge cases). labels Jul 29, 2022
@BenHenning BenHenning added Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Z-ibt Temporary label for Ben to keep track of issues he's triaged. issue_user_developer and removed Impact: Low Low perceived user impact (e.g. edge cases). labels Sep 15, 2022
@seanlip seanlip added enhancement End user-perceivable enhancements. and removed issue_type_infrastructure labels Mar 28, 2023
@MohitGupta121 MohitGupta121 added the Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. label Jun 16, 2023
@MohitGupta121 MohitGupta121 added Work: High It's not clear what the solution is. and removed Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. labels Jun 27, 2023
@BenHenning
Copy link
Member

I think this is completed at this point. We run buildifier as part of static_checks.sh:

oppia-android/scripts/static_checks.sh

Line 40 in 0ef7a46

bash scripts/buildifier_lint_check.sh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement End user-perceivable enhancements. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Work: High It's not clear what the solution is. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
[Team] Developer Workflow & Infrastructure - Android
Archived in project
Milestone
No milestone
Development

No branches or pull requests
5 participants
@seanlip @BenHenning @anandwana001 @MohitGupta121 @Broppia