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

Avoid erroneous developer commits to main branch #7585

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

berland
Copy link
Contributor

@berland berland commented Apr 5, 2024

Issue
Resolves developers forgetting to checkout a new branch before commiting.

Approach
pre-commit

Example of classical developer error that is guarded by this commit:

HAVB@AC-Y72WJR4W4M:~/projects/ert] main$ git commit -am "Avoid erroneous developer commits to main branch"
don't commit to branch...................................................Failed
- hook id: no-commit-to-branch
- exit code: 1
check json...........................................(no files to check)Skipped
check yaml...............................................................Passed
fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
ruff.................................................(no files to check)Skipped
ruff-format..........................................(no files to check)Skipped
clang-format.........................................(no files to check)Skipped
cmake-format.........................................(no files to check)Skipped
cmake-lint...........................................(no files to check)Skipped
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure tests pass locally (after every commit!)

When applicable

  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Create Backport PR to latest release

@berland berland added the release-notes:skip If there should be no mention of this in release notes label Apr 5, 2024
@berland berland self-assigned this Apr 5, 2024
@xjules
Copy link
Contributor

xjules commented Apr 8, 2024

Don't understand this tbh. What's the issue - is it to avoid force-push?

@berland
Copy link
Contributor Author

berland commented Apr 8, 2024

We should never commit to our local main, we should only ever do git commit when another branch is checked out.

When we commit to local main branch, we need to clean up with git reset --hard origin main.

This PR will make git commit fail unless you have checked out another branch than main

Copy link
Contributor

@jonathan-eq jonathan-eq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change the commit message to something like Have pre-commit protect main branch?

Developers should never commit when main branch is checked out.
Commits must happen on a branch, and then pull requests to main
branch.
@berland berland force-pushed the no-commit-to-main branch from 05b517d to 92d655c Compare April 9, 2024 10:38
@berland
Copy link
Contributor Author

berland commented Apr 9, 2024

Can we change the commit message to something like Have pre-commit protect main branch?

Fixed!

Copy link
Contributor

@jonathan-eq jonathan-eq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good!

@berland berland enabled auto-merge (rebase) April 9, 2024 10:44
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.20%. Comparing base (0e3f9b5) to head (92d655c).
Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7585      +/-   ##
==========================================
+ Coverage   85.18%   85.20%   +0.02%     
==========================================
  Files         383      383              
  Lines       23275    23293      +18     
  Branches      893      879      -14     
==========================================
+ Hits        19827    19847      +20     
+ Misses       3339     3335       -4     
- Partials      109      111       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@berland berland merged commit 6f49e28 into equinor:main Apr 9, 2024
37 checks passed
@berland berland deleted the no-commit-to-main branch May 29, 2024 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:skip If there should be no mention of this in release notes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants