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

Create CI task to fail PR if merge commit exists #12259

Closed
nrmancuso opened this issue Oct 6, 2022 · 7 comments · Fixed by #12343
Closed

Create CI task to fail PR if merge commit exists #12259

nrmancuso opened this issue Oct 6, 2022 · 7 comments · Fixed by #12343

Comments

@nrmancuso
Copy link
Member

nrmancuso commented Oct 6, 2022

Noticed at #12256 (comment), even though we have a bunch of checks in place for commit message, stale PR branch, etc., we do not fail CI for merge commits in PR branches.

I see github action https://github.com/greenled/no-merge-commits-check, maybe we can re-use the script in this repo and create a CI task to fail build if merge commits exist, and provide a message with link to https://checkstyle.org/beginning_development.html#Starting_Development .

@nrmancuso nrmancuso changed the title Create CI to fail PR if merge commit exists Create CI task to fail PR if merge commit exists Oct 6, 2022
@romani romani added the approved label Oct 6, 2022
@romani
Copy link
Member

romani commented Oct 6, 2022

We need to recheck why our commit message validation is not catching this.

@rnveach
Copy link
Member

rnveach commented Oct 6, 2022


private static final class OmitMergeCommitsIterator implements Iterator<RevCommit> {

Merge commits will fail CI as travis always works as a merge commit even if there is none in PR.

@nrmancuso
Copy link
Member Author

Sounds like shell script is the best way to handle this case then.

@romani
Copy link
Member

romani commented Oct 7, 2022

What is we made update in code and allow skip of merged only if some system variable is defined. And set it in Travis. All others will run without allowance of merge commit.

@rnveach
Copy link
Member

rnveach commented Oct 7, 2022

system variable is defined. And set it in Travis

I remember we had issues with different CIs and looking at git history similar to PR view. Travis was just the one I remember clearly.

@romani
Copy link
Member

romani commented Oct 7, 2022

At time we created this Junit validation Travis probably was the only CI we had.

@nrmancuso
Copy link
Member Author

I am on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants