[yast-devel] Protected branches at GitHub?
Hi, I have noticed a quite interesting GitHub feature which allows to protect the specified branches. A short summary from [1]: A protected branch: - Can't be force pushed - Can't be deleted - Can't have changes merged into it until required status checks pass - Can't have changes merged into it until required reviews are approved - Can't be edited or have files uploaded to it from the web See [1] and [2] for more details. AFAIK some developers use forks mainly because they are afraid of messing the original repositories. With protected branches that should not happen. Additionally it would strictly enforce PR reviews (we do that already so I see no problem with that). IMHO a nice feature, I'm just not sure which branches we could protect. Obviously the old maintenance branches, I'm not sure about the "master" branch, maybe it would be too strict. What do you think about it? Should we use this GitHub feature? We could use the agile approach, try it in a few selected repos and see how it works... Comments? [1] https://help.github.com/articles/about-protected-branches/ [2] https://help.github.com/articles/configuring-protected-branches/ -- Ladislav Slezák YaST Developer SUSE LINUX, s.r.o. Corso IIa Křižíkova 148/34 18600 Praha 8 -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On 16/09/16 09:12, Ladislav Slezak wrote:
Hi, Hi, I have noticed a quite interesting GitHub feature which allows to protect the specified branches. A short summary from [1]:
A protected branch: - Can't be force pushed - Can't be deleted - Can't have changes merged into it until required status checks pass - Can't have changes merged into it until required reviews are approved - Can't be edited or have files uploaded to it from the web
See [1] and [2] for more details.
AFAIK some developers use forks mainly because they are afraid of messing the original repositories. With protected branches that should not happen. I'm one of those developers :) Additionally it would strictly enforce PR reviews (we do that already so I see no problem with that). I agree. I think that we're quite strict about that (that's a good thing from my POV) so it should not be a problem. And if there's some special case we can always disable that feature for a while. IMHO a nice feature, I'm just not sure which branches we could protect. Obviously the old maintenance branches, I'm not sure about the "master" branch, maybe it would be too strict.
What do you think about it? Should we use this GitHub feature?
We could use the agile approach, try it in a few selected repos and see how it works...
Comments? I would give it a try in a few repositories and let's see how it works.
Regards, Imo -- Imobach González Sosa YaST Team at SUSE GmbH -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On Fri, 16 Sep 2016 10:55:52 +0100 Imobach Gonzalez Sosa <igonzalezsosa@suse.de> wrote:
Hi, Hi, I have noticed a quite interesting GitHub feature which allows to protect the specified branches. A short summary from [1]:
A protected branch: - Can't be force pushed - Can't be deleted - Can't have changes merged into it until required status checks pass - Can't have changes merged into it until required reviews are approved - Can't be edited or have files uploaded to it from the web
See [1] and [2] for more details.
AFAIK some developers use forks mainly because they are afraid of messing the original repositories. With protected branches that should not happen. I'm one of those developers :) Additionally it would strictly enforce PR reviews (we do that already so I see no problem with that). I agree. I think that we're quite strict about that (that's a good
On 16/09/16 09:12, Ladislav Slezak wrote: thing from my POV) so it should not be a problem. And if there's some special case we can always disable that feature for a while.
IMHO a nice feature, I'm just not sure which branches we could protect. Obviously the old maintenance branches, I'm not sure about the "master" branch, maybe it would be too strict.
What do you think about it? Should we use this GitHub feature?
We could use the agile approach, try it in a few selected repos and see how it works...
Comments? I would give it a try in a few repositories and let's see how it works.
I already put this item to restrospective board :) -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On 16.09.2016 10:12, Ladislav Slezak wrote:
AFAIK some developers use forks mainly because they are afraid of messing the original repositories. With protected branches that should not happen.
Additionally it would strictly enforce PR reviews (we do that already so I see no problem with that).
IMHO a nice feature, I'm just not sure which branches we could protect. Obviously the old maintenance branches, I'm not sure about the "master" branch, maybe it would be too strict.
We are all grown-ups. We all share responsibility for our subsystem. If we can't trust our own people, we have a problem. Don't overengineer stuff just because a tools offers you the possibility to do that. Just last week I needed to force-push yast2-storage to master, for example, after Josef had explained to me why we don't want to cherry-pick single commits from the SLE-12-SP2 branch to master. The explanation made sense, so I undid my cherry-picks. Of course, that meant doing the changes in my clone and then force-pushing it to master. Why would we want to take away this possibility to fix things? I don't see a real benefit. Kind regards -- Stefan Hundhammer <shundhammer@suse.de> YaST Developer SUSE Linux GmbH GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg) Maxfeldstr. 5, 90409 Nürnberg, Germany -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
Dne 19.9.2016 v 11:16 Stefan Hundhammer napsal(a):
We are all grown-ups. We all share responsibility for our subsystem. If we can't trust our own people, we have a problem.
No, it's not about trust or unexperienced developers. It about avoiding accident mistakes (typos on command line, overlooking, etc...). I believe everybody makes some mistakes from time to time (removing a wrong file, overwriting your changes, etc...), it's a kind of safety belt. Just like you should not work as root all the time even if you are a Linux master...
Don't overengineer stuff just because a tools offers you the possibility to do that.
Just last week I needed to force-push yast2-storage to master, for example, after Josef had explained to me why we don't want to cherry-pick single commits from the SLE-12-SP2 branch to master.
That's why I proposed to use this feature only for the old maintenance branches, master would be too strict probably, I agree with your point.
Why would we want to take away this possibility to fix things? I don't see a real benefit.
No, I do not want to block real fixes, I want to avoid possible accidental mistakes. If there is a real need to do a fix in a protected branch we can still temporarily disable this feature. As already proposed, let's try it few repositories and see how it works for us. If we find it annoying (disabling too often) we can disable it and stop using it. But for me it rather looks like a nice and potentially useful feature worth of trying... -- Ladislav Slezák YaST Developer SUSE LINUX, s.r.o. Corso IIa Křižíkova 148/34 18600 Praha 8 -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
Dne 20.9.2016 v 12:25 Ladislav Slezak napsal(a):
As already proposed, let's try it few repositories and see how it works for us. If we find it annoying (disabling too often) we can disable it and stop using it.
So far there was no problem related to the protected branches and as we discussed at the retrospective meeting we should enable it for more repositories. Therefore I have enabled it for all maintenance branches + the master branch for all yast/* GitHub repositories [1]. I wrote a simple script for doing that [2], it reported adding 2032 protected branches in total. As it was processed by a script I could over look some problem, if a wrong repository or a branch is protected just ping me and I'll fix that. Please enable it also for the libyui repositories (I do not have admin permissions there) and for the repos outside the "yast" organization (snapper, libstorage, linuxrc... I do not have a complete list and I do not know which branches should be affected). Either adapt my script or if it's only few repos you can do it manually at GitHub, see the documentation [3]. In addition to the documentation enable also the "Require pull request reviews before merging" and the "Include administrators" check boxes to have the same values everywhere. Note: For protected branches we could enable the "Require status checks to pass before merging" option, but as Travis/OBS is currently not much reliable I have not enabled that. We might reconsider this after switching to Docker based builds at Travis but for now it would be too strict as I think it could block merging a PR just because of some Travis or OBS issue which is not under our control. Note2: If you really need to do something what is not allowed with the protected branches and is a legitimate action (e.g. cleaning up a messed history) we can disable this feature temporarily and fix the issue. In that case just ping me or ask anybody with the GitHub admin rights for the repo. Enjoy! :-) Ladislav [1] I have disabled it back for the yast/burndown as PRs are not used there [2] https://github.com/yast/helper_scripts/pull/5 [3] https://help.github.com/articles/configuring-protected-branches/ -- Best Regards Ladislav Slezák Yast Developer -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On Tue, 20 Dec 2016 22:28:33 +0100 Ladislav Slezak <lslezak@suse.cz> wrote:
Dne 20.9.2016 v 12:25 Ladislav Slezak napsal(a):
As already proposed, let's try it few repositories and see how it works for us. If we find it annoying (disabling too often) we can disable it and stop using it.
So far there was no problem related to the protected branches and as we discussed at the retrospective meeting we should enable it for more repositories.
Therefore I have enabled it for all maintenance branches + the master branch for all yast/* GitHub repositories [1]. I wrote a simple script for doing that [2], it reported adding 2032 protected branches in total.
As it was processed by a script I could over look some problem, if a wrong repository or a branch is protected just ping me and I'll fix that.
JFYI: I disabled this feature for yast-translation as there is no reviews and weblate push it directly. Review should be done in weblate itself. Josef -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On Wed, Dec 21, 2016 at 03:41:35PM +0100, Josef Reidinger wrote:
On Tue, 20 Dec 2016 22:28:33 +0100 Ladislav Slezak <lslezak@suse.cz> wrote:
Dne 20.9.2016 v 12:25 Ladislav Slezak napsal(a):
As already proposed, let's try it few repositories and see how it works for us. If we find it annoying (disabling too often) we can disable it and stop using it.
So far there was no problem related to the protected branches and as we discussed at the retrospective meeting we should enable it for more repositories.
Therefore I have enabled it for all maintenance branches + the master branch for all yast/* GitHub repositories [1]. I wrote a simple script for doing that [2], it reported adding 2032 protected branches in total.
As it was processed by a script I could over look some problem, if a wrong repository or a branch is protected just ping me and I'll fix that.
JFYI: I disabled this feature for yast-translation as there is no reviews and weblate push it directly. Review should be done in weblate itself.
OK, the same should apply to libstorage and snapper. ciao Arvin -- Arvin Schnell, <aschnell@suse.com> Senior Software Engineer, Research & Development SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) Maxfeldstraße 5 90409 Nürnberg Germany -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
participants (5)
-
Arvin Schnell -
Imobach Gonzalez Sosa -
Josef Reidinger -
Ladislav Slezak -
Stefan Hundhammer