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

github: apply commit msg check to all commits #2155

Merged
merged 1 commit into from
Oct 10, 2024
Merged

github: apply commit msg check to all commits #2155

merged 1 commit into from
Oct 10, 2024

Conversation

mgouicem
Copy link
Contributor

@mgouicem mgouicem commented Oct 8, 2024

Current code checks the PR title and not the commit messages.
This PR:

  • renames the check from PR_check to commit_msg_check for more clarity
  • checks all commit messages in the PR instead of the PR title.

@github-actions github-actions bot added the devops Github automation label Oct 8, 2024
@mgouicem mgouicem force-pushed the mgouicem/main/commit_msg_fixup branch from 495e2ec to b46f019 Compare October 8, 2024 13:46
@mgouicem mgouicem marked this pull request as ready for review October 9, 2024 07:13
@mgouicem mgouicem requested a review from a team as a code owner October 9, 2024 07:13
theComputeKid
theComputeKid approved these changes Oct 9, 2024
View reviewed changes
Copy link
Contributor

@theComputeKid theComputeKid left a comment

Choose a reason for hiding this comment

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

Thanks!

@mgouicem mgouicem force-pushed the mgouicem/main/commit_msg_fixup branch from b46f019 to f646772 Compare October 9, 2024 09:53
@mgouicem mgouicem force-pushed the mgouicem/main/commit_msg_fixup branch from f646772 to 2b87bbd Compare October 10, 2024 11:40
@TaoLv
Copy link
Contributor

TaoLv commented Oct 10, 2024

checks all commit messages in the PR instead of the PR title.

I see there are a few PRs are merged with squash and by default, the PR titles are used as the final commit message. Do we want to force rebase and merge?

@mgouicem
Copy link
Contributor Author

mgouicem commented Oct 10, 2024 •
edited
Loading

checks all commit messages in the PR instead of the PR title.

I see there are a few PRs are merged with squash and by default, the PR titles are used as the final commit message. Do we want to force rebase and merge?

I guess there are two things here:

  • it should help guide new developers to CONTRIBUTING.md, and encourage them to properly format their commit messages
  • for devs bypassing branch protection, it serves as a reminder to squash and properly name the commit message.

And yes, all commits should be rebased on top of target branch to keep a linear history (no merge commit).

@mgouicem mgouicem merged commit b9c3c4f into uxlfoundation:main Oct 10, 2024
11 of 17 checks passed
@mgouicem mgouicem deleted the mgouicem/main/commit_msg_fixup branch October 11, 2024 13:35
@vpirogov vpirogov added this to the v3.7 milestone Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theComputeKid theComputeKid theComputeKid approved these changes

Assignees
No one assigned
Labels
devops Github automation
Projects
None yet
Milestone
v3.7
Development

Successfully merging this pull request may close these issues.
4 participants
@mgouicem @TaoLv @theComputeKid @vpirogov