protect the master and maven-3.9.x branches to prevent accidental commits by elharo · Pull Request #1035 · apache/maven

elharo · 2023-03-05T12:55:07Z

The goal is to keep committers (i.e. me) from accidentally pushing straight into master. I think this does that, but I've only done this before through the github UI so please review carefully.

michael-o · 2023-03-05T13:02:19Z

Well, this will also add overhead if you fix a typo and are safe to push directly.

cstamas · 2023-03-05T15:55:19Z

And this prevents release plugin as well (when doing a release from master)?

slawekjaranowski

Although I like PR review I wouldn't to do such strict restrictions.

all PR will be must past one by one and each will be must rebased with the latest master before merge
all even simple fix (typo) will be must pass by whole machine
we don't have a consesus for RTC
release from master will be not possible

hboutemy · 2023-03-05T16:58:24Z

And this prevents release plugin as well (when doing a release from master)?

this one would be a blocker to me

olamy · 2023-03-05T21:06:14Z

And this prevents release plugin as well (when doing a release from master)?

this one would be a blocker to me

just saying nothing really prevents. create a branch release/4.0.0-alpha or release/3.9.2 or release/3.8.10 when starting the release process and then release from this branch. As is we do not block people merging to main branches during release process...
Then once the release is done just create a PR from the release/xxx branch.
The current way of releasing is a bit of legacy from svn days,

michael-o · 2023-03-05T21:10:36Z

And this prevents release plugin as well (when doing a release from master)?

this one would be a blocker to me

just saying nothing really prevents. create a branch release/4.0.0-alpha or release/3.9.2 or release/3.8.10 when starting the release process and then release from this branch. As is we do not block people merging to main branches during release process... Then once the release is done just create a PR from the release/xxx branch. The current way of releasing is a bit of legacy from svn days,

Bah, even more work...

olamy · 2023-03-05T21:49:16Z

And this prevents release plugin as well (when doing a release from master)?

this one would be a blocker to me

just saying nothing really prevents. create a branch release/4.0.0-alpha or release/3.9.2 or release/3.8.10 when starting the release process and then release from this branch. As is we do not block people merging to main branches during release process... Then once the release is done just create a PR from the release/xxx branch. The current way of releasing is a bit of legacy from svn days,

Bah, even more work...

says the one is asking people to create jira for everything :P
actually it's just 2 command line:

git checkout -b release/....
when done with the release from from the branch: gh pr create -B

I just find the "hey guys hold on your merge for a week or two I'm releasing so nobody touch anything" a bit archaic svn/cvs style :)
But this was just a comment from the peanut gallery. And nothing prevent someone to do it at the end of day.

BTW I'm not for ban direct push but not for this release reason.

michael-o · 2023-03-05T21:51:26Z

And this prevents release plugin as well (when doing a release from master)?

this one would be a blocker to me

just saying nothing really prevents. create a branch release/4.0.0-alpha or release/3.9.2 or release/3.8.10 when starting the release process and then release from this branch. As is we do not block people merging to main branches during release process... Then once the release is done just create a PR from the release/xxx branch. The current way of releasing is a bit of legacy from svn days,

Bah, even more work...

says the one is asking people to create jira for everything :P actually it's just 2 command line:
* git checkout -b release/....

* when done with the release from from the branch: `gh pr create -B`
I just find the "hey guys hold on your merge for a week or two I'm releasing so nobody touch anything" a bit archaic svn/cvs style :) But this was just a comment from the peanut gallery. And nothing prevent someone to do it at the end of day.

BTW I'm not for ban direct push but not for this release reason.

...and how do you want to reconcile two diverging branches back to a linear history?

olamy · 2023-03-05T21:57:32Z

And this prevents release plugin as well (when doing a release from master)?

this one would be a blocker to me

just saying nothing really prevents. create a branch release/4.0.0-alpha or release/3.9.2 or release/3.8.10 when starting the release process and then release from this branch. As is we do not block people merging to main branches during release process... Then once the release is done just create a PR from the release/xxx branch. The current way of releasing is a bit of legacy from svn days,

Bah, even more work...

says the one is asking people to create jira for everything :P actually it's just 2 command line:
* git checkout -b release/....

* when done with the release from from the branch: `gh pr create -B`
I just find the "hey guys hold on your merge for a week or two I'm releasing so nobody touch anything" a bit archaic svn/cvs style :) But this was just a comment from the peanut gallery. And nothing prevent someone to do it at the end of day.
BTW I'm not for ban direct push but not for this release reason.
...and how do you want to reconcile two diverging branches back to a linear history?

I don't see the problem here. Can you explain?

You have maven-3.8.x ready for 3.8.25 release.
so create a branch release/3.8.25, then start the release process from this branch.
At the end, you can create a PR from release/3.8.25 to maven-3.8.x or just increment version in maven-3.8.x (as long as you have the tag, does the commit by the m-release-p really matter?)

michael-o · 2023-03-05T21:59:16Z

And this prevents release plugin as well (when doing a release from master)?

this one would be a blocker to me

just saying nothing really prevents. create a branch release/4.0.0-alpha or release/3.9.2 or release/3.8.10 when starting the release process and then release from this branch. As is we do not block people merging to main branches during release process... Then once the release is done just create a PR from the release/xxx branch. The current way of releasing is a bit of legacy from svn days,

Bah, even more work...

says the one is asking people to create jira for everything :P actually it's just 2 command line:
* git checkout -b release/....

* when done with the release from from the branch: `gh pr create -B`
I just find the "hey guys hold on your merge for a week or two I'm releasing so nobody touch anything" a bit archaic svn/cvs style :) But this was just a comment from the peanut gallery. And nothing prevent someone to do it at the end of day.
BTW I'm not for ban direct push but not for this release reason.
...and how do you want to reconcile two diverging branches back to a linear history?
I don't see the problem here. Can you explain?

You have maven-3.8.x ready for 3.8.25 release. so create a branch release/3.8.25, then start the release process from this branch. At the end, you can create a PR from release/3.8.25 to maven-3.8.x or just increment version in maven-3.8.x (as long as you have the tag, does the commit by the m-release-p really matter?)

What if master has developed from then? A ff-merge isn't possible anymore, no?

olamy · 2023-03-05T23:58:42Z

I don't see the problem here. Can you explain?
You have maven-3.8.x ready for 3.8.25 release. so create a branch release/3.8.25, then start the release process from this branch. At the end, you can create a PR from release/3.8.25 to maven-3.8.x or just increment version in maven-3.8.x (as long as you have the tag, does the commit by the m-release-p really matter?)

What if master has developed from then? A ff-merge isn't possible anymore, no?

and so what is the problem?
the tag is there. it's the most important of a release. then just bump version in the main branch.

elharo · 2025-06-24T11:51:46Z

Bringing this one back up. Accidental and deliberate pushes to the master branch without review continue to happen and this is not good for security or reliability. I think @olamy has clearly indicated how the release process can work following this change.

michael-o · 2025-06-24T12:08:32Z

Bringing this one back up. Accidental and deliberate pushes to the master branch without review continue to happen and this is not good for security or reliability. I think @olamy has clearly indicated how the release process can work following this change.

It should be obviously summarized because no one remembers. Also note that the "Merge Button" on GH does not replace a merge in the terminal especially because it cannot set the proper committer email address.

elharo · 2025-06-24T12:22:45Z

I think @olamy can explain better than me, but briefly the procedure would be to make a branch for the release, release from the branch, and then increment the version in master, manually if necessary.

michael-o · 2025-06-24T12:38:08Z

I think @olamy can explain better than me, but briefly the procedure would be to make a branch for the release, release from the branch, and then increment the version in master, manually if necessary.

Overhead for a volunteer

elharo · 2025-06-24T13:58:01Z

Yes, it is extra overhead. IMHO it's worth it. The current situation is unacceptably vulnerable to supply chain attacks. Today any committer account can unilaterally install a crypto miner, backdoor, or other malware into Maven with a strong chance that it will go unnoticed before release.

michael-o · 2025-06-24T14:11:52Z

Yes, it is extra overhead. IMHO it's worth it. The current situation is unacceptably vulnerable to supply chain attacks. Today any committer account can unilaterally install a crypto miner, backdoor, or other malware into Maven with a strong chance that it will go unnoticed before release.

Maven for the paranoid.

elharo · 2025-06-24T15:00:49Z

It's not like these problems haven't happened before multiple times in multiple projects. When people really are out to get you, paranoia is the rational state of mind.

slawekjaranowski · 2025-06-24T15:24:32Z

Yes, it is extra overhead. IMHO it's worth it. The current situation is unacceptably vulnerable to supply chain attacks. Today any committer account can unilaterally install a crypto miner, backdoor, or other malware into Maven with a strong chance that it will go unnoticed before release.

even if we have a protected master (or other branches) I will do release by creating branch for release like release/x.x.x than add what I want to this branch and do a release ...

even more I can prepare own binary artifacts which will be released .... if nobody check reproducible build result ...

so protected branches don't protect us by supply chain attacks and so on

elharo · 2025-06-24T18:30:12Z

You might be able to do that. I and other non-PMC committers can't. And generally a release requires a review and a vote. Possibly that's a formality and you could skip the vote but the release would be noticed, and likely someone would take a closer look. And usually someone does check the reproducible build result.

OTOH you or I certainly could commit something to master with a reasonable likelihood no one would pay any attention. We can close that hole and we should. Let's not let the perfect be the enemy of the good.

olamy · 2025-06-24T21:09:36Z

I do not see this fixing any security problems...
The only potential problem fixed is that people merging to master while someone is building a release.
Well, on one side we want to have only PR workflow, but on the other side we keep the option to push directly to the main branch. Sounds a bit contradictory :)
Frankly, I don't have a strong opinion. I'm happy to be still able to push directly to master and so not have useless changes as PR, which will be in the release notes as maintenance.

olamy · 2025-06-24T21:13:07Z

But if doing so, maven-3.9.x must be protected too
The required approval must also be discussed, as it's not what we are used to doing.

elharo · 2025-06-24T21:19:58Z

To be clear, I want a PR only workflow, full stop.

I suppose if we were using some non-Github system, then there would be other non-PR mechanisms for guaranteeing code review, but two person sign-off on all changes is standard practice for good reason. Given that we're using github though, PRs are simply how that is done.

And yes, maven-3.9.x should be protected too.

olamy · 2025-06-24T21:31:13Z

To be clear, I want a PR only workflow, full stop.

Well such change can only come from what "we" want (maven dev community) and not from what "you" want (a single person opinion).
As far as I can see from this PR this doesn't look to reach some consensus.
So this can only be discussed and maybe voted on ML (as a democratic decision coming from the community)

I suppose if we were using some non-Github system, then there would be other non-PR mechanisms for guaranteeing code review, but two person sign-off on all changes is standard practice for good reason. Given that we're using github though, PRs are simply how that is done.

And yes, maven-3.9.x should be protected too.

elharo · 2025-06-25T11:10:43Z

I'm happy to discuss on the mailing list. I do think this is important to get right. I don't want Maven to to become the next cautionary tale on Hacker News.

elharo · 2025-06-25T11:35:06Z

Re consensus, https://maven.apache.org/developers/conventions/github.html states:

"We should always provide changes by Pull Request. Direct commits will be not visible in Milestones issues list nor Release Notes."

And I did not write the PR that put that language there. :-)

olamy · 2025-06-25T22:01:18Z

Not everything has to be in release notes. Some changes are pure cosmetic and doesn't affect users and turns to be useless noise in release notes

olamy · 2025-06-25T22:53:46Z

Re consensus, https://maven.apache.org/developers/conventions/github.html states:

"We should always provide changes by Pull Request. Direct commits will be not visible in Milestones issues list nor Release Notes."

And I did not write the PR that put that language there. :-)

I'm not an English native speaker, but I think there is a difference between should and must.

Re consensus, https://maven.apache.org/developers/conventions/github.html states:

"We should always provide changes by Pull Request. Direct commits will be not visible in Milestones issues list nor Release Notes."

And I did not write the PR that put that language there. :-)

https://maven.apache.org/developers/conventions/git.html

For committers
Committers may, of course, commit directly to the ASF repositories. For complex changes, you may find it valuable to make a pull request at github to make it easier to collaborate with others.

gnodet

AI Code Review — PR #1035

Verdict: 💬 Superseded — recommend close

This PR proposed adding protected_branches for master and maven-3.9.x with strict status checks and required reviews. However, branch protection has since been enabled through other PRs and direct commits.

Status

The current .asf.yaml on master already includes comprehensive branch protection:

protected_branches:
  master: {}
  maven-4.0.x: {}
  maven-3.10.x: {}
  maven-3.9.x: {}
  maven-3.8.x: {}
  maven-3.0.x: {}
  maven-2.2.x: {}
  pre-reset-master: {}

This covers 8 branches (vs. the 2 proposed here). The community chose empty {} rules (preventing force-pushes and branch deletion) rather than the stricter required_status_checks and required_pull_request_reviews proposed here — as discussed in the PR comments, the stricter settings raised governance concerns about release workflow overhead.

Recommendation

Close this PR as superseded. The core goal (protecting key branches) is already accomplished. If stricter settings (required reviews, up-to-date branches) are desired later, that should be proposed via the dev mailing list with a fresh PR against the current .asf.yaml.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

Reviewed 3 PRs: apache#1511 (COMMENT), apache#1035 (COMMENT, superseded), apache#964 (COMMENT). 30 total reviews posted. ~29 PRs remaining. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

elharo · 2026-07-10T12:45:08Z

OK sounds good. I'll close this. But I am going to check if I can still commit straight to master without review (using some innocuous typo fix, they're easy to find) and if I can I'll ask for stronger protections.

protect the master branch to prevent accidental commits

f0423e5

elharo requested review from hboutemy, khmarbaise and rfscholte March 5, 2023 12:55

slawekjaranowski requested changes Mar 5, 2023

View reviewed changes

hboutemy force-pushed the master branch from a075afa to 6e50886 Compare March 24, 2024 21:16

merge master

93c01b3

Protect maven-3.9.x branch

d85689e

elharo changed the title ~~protect the master branch to prevent accidental commits~~ protect the master and maven-3.9.x branches to prevent accidental commits Jun 24, 2025

slawekjaranowski mentioned this pull request Mar 28, 2026

Clenups old - stale branches #11858

Open

gnodet reviewed Jul 10, 2026

View reviewed changes

elharo closed this Jul 10, 2026

elharo mentioned this pull request Jul 10, 2026

Verify direct commits to master are now blocked #12462

Open

Uh oh!

Conversation

elharo commented Mar 5, 2023

Uh oh!

michael-o commented Mar 5, 2023

Uh oh!

cstamas commented Mar 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slawekjaranowski left a comment

Choose a reason for hiding this comment

Uh oh!

hboutemy commented Mar 5, 2023

Uh oh!

olamy commented Mar 5, 2023

Uh oh!

michael-o commented Mar 5, 2023

Uh oh!

olamy commented Mar 5, 2023

Uh oh!

michael-o commented Mar 5, 2023

Uh oh!

olamy commented Mar 5, 2023

Uh oh!

michael-o commented Mar 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

olamy commented Mar 5, 2023

Uh oh!

elharo commented Jun 24, 2025

Uh oh!

michael-o commented Jun 24, 2025

Uh oh!

elharo commented Jun 24, 2025

Uh oh!

michael-o commented Jun 24, 2025

Uh oh!

elharo commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michael-o commented Jun 24, 2025

Uh oh!

elharo commented Jun 24, 2025

Uh oh!

slawekjaranowski commented Jun 24, 2025

Uh oh!

elharo commented Jun 24, 2025

Uh oh!

olamy commented Jun 24, 2025

Uh oh!

olamy commented Jun 24, 2025

Uh oh!

elharo commented Jun 24, 2025

Uh oh!

olamy commented Jun 24, 2025

Uh oh!

elharo commented Jun 25, 2025

Uh oh!

elharo commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

olamy commented Jun 25, 2025

Uh oh!

olamy commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

AI Code Review — PR #1035

Status

Recommendation

Uh oh!

elharo commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

cstamas commented Mar 5, 2023 •

edited

Loading

michael-o commented Mar 5, 2023 •

edited

Loading

elharo commented Jun 24, 2025 •

edited

Loading

elharo commented Jun 25, 2025 •

edited

Loading

olamy commented Jun 25, 2025 •

edited

Loading