You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2022/10/13 11:44:51 UTC
[GitHub] [netbeans] mbien opened a new pull request, #4785: enable branch protection for master.
mbien opened a new pull request, #4785:
URL: https://github.com/apache/netbeans/pull/4785
should we:
- enable force push protection for delivery too?
- add reviewer requirement for master?
we have to watch out to not lock us out when enabling any of the more advanced features.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#issuecomment-1278936042
The main reason why i opened this draft is because branch protection came up in a discussion recently. So i thought before I forget about it, I read the ASF doc and check what is possible.
> > enable force push protection for delivery too?
>
> Please don't! `delivery` is a transient branch that we do occasionally need to force push into a suitable state.
i had the same feeling that is why I left it commented out.
> > add reviewer requirement for master?
>
> anyone trusted to merge should be able to make the judgement on what level of review is required for any change. And anyway, code can always be vetoed and/or reverted after the fact if necessary.
sure
so essentially we can reduce it to `master: {}` and leave everything else unconfigured/disabled. Although having green checkmarks would be a interesting requirement for later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] ebarboni commented on pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
ebarboni commented on PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#issuecomment-1278887831
Not sure what it involve
this means I will no more be able to change the NOTICE file date each year by direct commit to master :p ?
If that I would be able to merge a PR myself :D
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#issuecomment-1307277525
> I'm not neutral about a green CI requirement for merging (once tests are stable). That might be better achieved by a notice email and convention rather than branch protection.
I probably misinterpreted something since I thought you suggested the green checkmark requirement somewhere. But if you don't like that idea either - even better - lets drop it from the list :)
> an infra ticket requirement would be a larger hurdle than necessary!
oh I didn't mean it like that. I meant that in an event we would be truly locked out we could:
- simply push to master and turn that thing off
- if everything fails ask infra to turn it off again (but tbh I don't know how we could end up that way)
I didn't mean to add infra to the process of merging a PR which has CI failing.
But this is theoretical. Since given that we don't use `pull_request_target`, I can't see right now how we could lock ourselves out.
So is force push protection to master something we all want or should I close this whole PR since that would be the last open point as far as I see?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#issuecomment-1279352914
repurposed the cleanup job as CI_GREEN flag
- will be red on cancel or on any job failure
- will be also green on ci:no-build
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#issuecomment-1303902856
@matthiasblaesing I updated the PR description with more details regarding lock-out. Do you still have concerns regarding a green CI requirement? I believe a complete CI failure is basically a release showstopper. We probably would have to wait till it is resolved before continuing - I don't think anyone would feel comfortable to use github to merge PRs as long CI is not available?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#issuecomment-1278923154
> Not sure what it involve this means I will no more be able to change the NOTICE file date each year by direct commit to master
Well, that's not necessarily a bad thing. I mean, I usually run on all ASF checkouts `git remote set-url --push upstream DISABLED` anyway to at least make it more difficult for myself to accidentally push to the wrong place! :smile:
> enable force push protection for delivery too?
Please don't! `delivery` is a transient branch that we do occasionally need to force push into a suitable state.
> add reviewer requirement for master?
I think that's better as a social requirement rather than a technical one - eg. anyone trusted to merge should be able to make the judgement on what level of review is required for any change. And anyway, code can always be vetoed and/or reverted after the fact if necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#discussion_r995710454
##########
.asf.yaml:
##########
@@ -31,6 +33,23 @@ github:
squash: true
merge: true
rebase: true
+ protected_branches:
+ # no force push to master
+ master:
+# required_pull_request_reviews:
+# required_approving_review_count: 1
+# dismiss_stale_reviews: false
+ required_status_checks:
+ # strict means "Require branches to be up to date before merging".
+ strict: false
+ # list of names of checks that must pass
+# contexts:
+# - Commit Validation on Linux/JDK 11
Review Comment:
@neilcsmith-net the status check looks at the `contexts` list and requires all checks in that list to be green. We would need some meta-job for this to handle edge cases like `ci:no-build`. Otherwise we could have used "Cleanup Workflow Artifacts". Since if that one is green, everything is fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#issuecomment-1279434785
> I'm against required checks for merges to master. We know, that CI systems fail. I've seen it happen on appveyor and travis and I'm sure that github action will fail soner or later too. If that happens we are locked out of our own repository because we can't remove the protections, because that would require a successful check execution.
you can make the check in question pass in any PR if something happens.
```
- name: Final Check
if: ${{ always() && (contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'failure')) }}
run: exit 1
```
if this is commented out the check will be green
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien merged pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien merged PR #4785:
URL: https://github.com/apache/netbeans/pull/4785
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#discussion_r995899080
##########
.asf.yaml:
##########
@@ -31,6 +33,23 @@ github:
squash: true
merge: true
rebase: true
+ protected_branches:
+ # no force push to master
+ master:
+# required_pull_request_reviews:
+# required_approving_review_count: 1
+# dismiss_stale_reviews: false
+ required_status_checks:
+ # strict means "Require branches to be up to date before merging".
+ strict: false
+ # list of names of checks that must pass
+# contexts:
+# - Commit Validation on Linux/JDK 11
Review Comment:
I am going to try to make this work somehow. (but yes I added it primarily to check new contributors, I didn't check committer/PMC emails during reviews so far).
The "check" doesn't do anything btw. All it does is to print the details of the last 10 commits.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#issuecomment-1307783038
- removed everything except master branch force push protection
- moved the parts which make the paperwork job always run over to #4817
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#issuecomment-1279423457
I'm against required checks for merges to master. We know, that CI systems fail. I've seen it happen on appveyor and travis and I'm sure that github action will fail soner or later too. If that happens we are locked out of our own repository because we can't remove the protections, because that would require a successful check execution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on a diff in pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on code in PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#discussion_r995808141
##########
.asf.yaml:
##########
@@ -31,6 +33,23 @@ github:
squash: true
merge: true
rebase: true
+ protected_branches:
+ # no force push to master
+ master:
+# required_pull_request_reviews:
+# required_approving_review_count: 1
+# dismiss_stale_reviews: false
+ required_status_checks:
+ # strict means "Require branches to be up to date before merging".
+ strict: false
+ # list of names of checks that must pass
+# contexts:
+# - Commit Validation on Linux/JDK 11
Review Comment:
Ah, OK, your meta-job comment now makes sense to me! Mind you, we should possibly do some basic paperwork checks (eg. commit info) for all merged PRs? `ci:no-build` could mean skip the build and dependent checks rather than no actions at all?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#discussion_r994549953
##########
.asf.yaml:
##########
@@ -31,6 +33,23 @@ github:
squash: true
merge: true
rebase: true
+ protected_branches:
+ # no force push to master
+ master:
+# required_pull_request_reviews:
+# required_approving_review_count: 1
+# dismiss_stale_reviews: false
+ required_status_checks:
+ # strict means "Require branches to be up to date before merging".
+ strict: false
+ # list of names of checks that must pass
+# contexts:
+# - Commit Validation on Linux/JDK 11
Review Comment:
we could probably set up a meta-job which serves as check for all other jobs, if we decide to turn this on. (and on halloween we add travis to the list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#discussion_r996050785
##########
.asf.yaml:
##########
@@ -31,6 +33,16 @@ github:
squash: true
merge: true
rebase: true
+ # no force push to listed branches
+ protected_branches:
+ master:
+ required_status_checks:
+ # strict means "Require branches to be up to date before merging".
+ strict: false
+ # list of names of PR checks that must pass
+ contexts:
+ - CI_GREEN
Review Comment:
this should enable force push protection for master and enable the status check requirement which is represented by the CI_GREEN job.
I am scared to add travis there. If something doesn't work or the check "Travis CI - Pull Request " is not recognized for some reason (or a travis update renames it) we lock ourself out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#discussion_r996050785
##########
.asf.yaml:
##########
@@ -31,6 +33,16 @@ github:
squash: true
merge: true
rebase: true
+ # no force push to listed branches
+ protected_branches:
+ master:
+ required_status_checks:
+ # strict means "Require branches to be up to date before merging".
+ strict: false
+ # list of names of PR checks that must pass
+ contexts:
+ - CI_GREEN
Review Comment:
this should enable force push protection for master and enable the status check requirement which is represented by the CI_GREEN job.
I am scared to add travis there. If something doesn't work or the check "Travis CI - Pull Request " is not recognized for some reason we lock ourself out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#issuecomment-1307323013
> I probably misinterpreted something since I thought you suggested the green checkmark requirement somewhere.
I'm in favour (with stable tests) of a general requirement for tests to be green before merging. We have had things merged in the past that were assumed to not be causes of test failures and turned out later to be a problem. So I don't entirely agree it's solving a problem we don't have. On the other hand, that's probably better solved by communication.
The admin access UI to bypass branch protection (which afaik we don't have) [like shown here](https://stackoverflow.com/questions/73348721/github-branch-protection-disable-bypass) would be great IMO, but without that it's probably overkill, for now at least. Let's see how we do without it.
I'm +1 to protecting against force push (then again, to an extent I'm in favour of disallowing direct push completely, as we've had things accidentally pushed before now).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#issuecomment-1305986390
@matthiasblaesing tbh I am fairly neutral regarding the green CI requirement too. @neilcsmith-net wanted the Paperwork job to always run (taken from other discussions) so that we can put more checks on it. So I thought if we do that anyway, we may as well enable the green checkmark requirement. I do agree that probably nobody will merge a PR which is not green - at least not intentionally. Question is the "not intentionally" a risk worth thinking about or not.
Do we have consensus on the force push protection to master (@ebarboni you ok with this too?)? If yes I'll update this PR to do only:
```yaml
github:
protected_branches:
master: {}
```
which should do the trick according to the doc
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#issuecomment-1304628234
> @matthiasblaesing I updated the PR description with more details regarding lock-out. Do you still have concerns regarding a green CI requirement? I believe a complete CI failure is basically a release showstopper. We probably would have to wait till it is resolved before continuing - I don't think anyone would feel comfortable to use github to merge PRs as long CI is not available?
I think this solves a problem we don't have and can cause issues. If there is consensus to enable branch protection do it, I won't +1 this, but also won't hold it back. I bet on having gitbox as the final rescue access for situations that require it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#issuecomment-1307075183
I'm not neutral about a green CI requirement for merging (once tests are stable). That might be better achieved by a notice email and convention rather than branch protection. If PMC members had bypass branch protection permission, I'd say enable - an extra click box as a little extra hurdle to take responsibility that the checks need ignoring is one thing, an infra ticket requirement would be a larger hurdle than necessary!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#issuecomment-1278939663
I'm in favour of required status checks though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#discussion_r995710454
##########
.asf.yaml:
##########
@@ -31,6 +33,23 @@ github:
squash: true
merge: true
rebase: true
+ protected_branches:
+ # no force push to master
+ master:
+# required_pull_request_reviews:
+# required_approving_review_count: 1
+# dismiss_stale_reviews: false
+ required_status_checks:
+ # strict means "Require branches to be up to date before merging".
+ strict: false
+ # list of names of checks that must pass
+# contexts:
+# - Commit Validation on Linux/JDK 11
Review Comment:
@neilcsmith-net status checks look at the `contexts` list and require all checks of that particular name to be green. We would need some meta-job for this to handle edge cases like `ci:no-build`. Otherwise we could have used "Cleanup Workflow Artifacts". Since if that one is green, everything is fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#discussion_r996050785
##########
.asf.yaml:
##########
@@ -31,6 +33,16 @@ github:
squash: true
merge: true
rebase: true
+ # no force push to listed branches
+ protected_branches:
+ master:
+ required_status_checks:
+ # strict means "Require branches to be up to date before merging".
+ strict: false
+ # list of names of PR checks that must pass
+ contexts:
+ - CI_GREEN
Review Comment:
this should give up force push protection for master and enable the status check requirement which is represented by the CI_GREEN job.
I am scared to add travis there. If something doesn't work or the check is not recognized for some reason we lock ourself out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#discussion_r994549953
##########
.asf.yaml:
##########
@@ -31,6 +33,23 @@ github:
squash: true
merge: true
rebase: true
+ protected_branches:
+ # no force push to master
+ master:
+# required_pull_request_reviews:
+# required_approving_review_count: 1
+# dismiss_stale_reviews: false
+ required_status_checks:
+ # strict means "Require branches to be up to date before merging".
+ strict: false
+ # list of names of checks that must pass
+# contexts:
+# - Commit Validation on Linux/JDK 11
Review Comment:
we could probably set up a meta-job which serves as check for all other jobs, if we decide to turn this on.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#discussion_r996175750
##########
.asf.yaml:
##########
@@ -31,6 +33,23 @@ github:
squash: true
merge: true
rebase: true
+ protected_branches:
+ # no force push to master
+ master:
+# required_pull_request_reviews:
+# required_approving_review_count: 1
+# dismiss_stale_reviews: false
+ required_status_checks:
+ # strict means "Require branches to be up to date before merging".
+ strict: false
+ # list of names of checks that must pass
+# contexts:
+# - Commit Validation on Linux/JDK 11
Review Comment:
i think it is working. Paperwork will now always run. The parts which require an artifact will be skipped.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#discussion_r995835503
##########
.asf.yaml:
##########
@@ -31,6 +33,23 @@ github:
squash: true
merge: true
rebase: true
+ protected_branches:
+ # no force push to master
+ master:
+# required_pull_request_reviews:
+# required_approving_review_count: 1
+# dismiss_stale_reviews: false
+ required_status_checks:
+ # strict means "Require branches to be up to date before merging".
+ strict: false
+ # list of names of checks that must pass
+# contexts:
+# - Commit Validation on Linux/JDK 11
Review Comment:
the second half of the paperwork job requires the build artifact for verify-libs-and-licenses, check-sigtests-release and check-sigtests-release. So we would have to split it to have one part of it before/without the build and the other as normal secondary, post-build job.
Although I am wondering what happens if B depends on A, A is skipped and B is set to `always()`. If it still runs we might have our way out without having to split the job. Something I can test in my sandbox.
As a workaround we could simply avoid `ci:no-build` on PRs from new contributors so that we have the option to inspect the papers. (I mean inspection via checkout is always possible, thats how I used to do it)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on a diff in pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on code in PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#discussion_r995848713
##########
.asf.yaml:
##########
@@ -31,6 +33,23 @@ github:
squash: true
merge: true
rebase: true
+ protected_branches:
+ # no force push to master
+ master:
+# required_pull_request_reviews:
+# required_approving_review_count: 1
+# dismiss_stale_reviews: false
+ required_status_checks:
+ # strict means "Require branches to be up to date before merging".
+ strict: false
+ # list of names of checks that must pass
+# contexts:
+# - Commit Validation on Linux/JDK 11
Review Comment:
> PRs from new contributors so that we have the option to inspect the papers.
That suggests it's new contributors rather than existing committers / PMC members whose eg. invalid noreply email addresses in commits needs to be flagged up! :wink:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#issuecomment-1307344469
> I'm +1 to protecting against force push (then again, to an extent I'm in favour of disallowing direct push completely, as we've had things accidentally pushed before now).
i don't think there is a direct push protection setting. Doc only mentions [force push](https://cwiki.apache.org/confluence/display/INFRA/Git+-+.asf.yaml+features). If the doc is up2date, force push protection is all we can get right now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4785: enable branch protection for master.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4785:
URL: https://github.com/apache/netbeans/pull/4785#discussion_r996050785
##########
.asf.yaml:
##########
@@ -31,6 +33,16 @@ github:
squash: true
merge: true
rebase: true
+ # no force push to listed branches
+ protected_branches:
+ master:
+ required_status_checks:
+ # strict means "Require branches to be up to date before merging".
+ strict: false
+ # list of names of PR checks that must pass
+ contexts:
+ - CI_GREEN
Review Comment:
this should enable force push protection for master and enable the status check requirement which is represented by the CI_GREEN job.
~~I am scared to add travis there. If something doesn't work or the check "Travis CI - Pull Request " is not recognized for some reason (or a travis update renames it) we lock ourself out.~~ <- no longer relevant since travis is soon gone anyway
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists