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

protect master branches except for the pecl repos against force pushes #4

Open
wants to merge 2 commits into
from

Conversation

4 participants
@Tyrael
Member

Tyrael commented Aug 9, 2015

as discussed on the mailing list it would be nice if not everybody can force push for every other repo but php-src.git.
@johannes mentioned that we should allow force pushes for the pecl repos so what I came up with is to restrict force pushes by default for the master branch on every repo except those with a name starting with pecl/.
I tried to test my changes, but testing a pre-receive hooks is a PITA manually, so it would be nice if somebody could review my change before merging it.

Show outdated Hide outdated hooks/pre-receive
if (isset($restrictedBranches[$repo_name])) {
$restricted = array_filter($restrictedBranches[$repo_name],
$restricted += array_filter($restrictedBranches[$repo_name],

This comment has been minimized.

@salathe

salathe Aug 9, 2015

Contributor

The + operator will give the wrong behaviour when $restricted is not empty, the first filtered value will not be present in the resulting array. You'll want to use array_merge() instead or append the master ref later on.

@salathe

salathe Aug 9, 2015

Contributor

The + operator will give the wrong behaviour when $restricted is not empty, the first filtered value will not be present in the resulting array. You'll want to use array_merge() instead or append the master ref later on.

This comment has been minimized.

@Tyrael

Tyrael Aug 9, 2015

Member

ofc, thanks!

@Tyrael

Tyrael Aug 9, 2015

Member

ofc, thanks!

@bwoebi

This comment has been minimized.

Show comment
Hide comment
@bwoebi

bwoebi Aug 9, 2015

You want to eventually exempt the repos listed under misc (like on http://git.php.net) too

bwoebi commented Aug 9, 2015

You want to eventually exempt the repos listed under misc (like on http://git.php.net) too

@Tyrael

This comment has been minimized.

Show comment
Hide comment
@Tyrael

Tyrael Aug 9, 2015

Member

@bwoebi why/what repo? misc is not a homogen group, looking through the repos not sure if we want to excempt any of those.

Member

Tyrael commented Aug 9, 2015

@bwoebi why/what repo? misc is not a homogen group, looking through the repos not sure if we want to excempt any of those.

@bwoebi

This comment has been minimized.

Show comment
Hide comment
@bwoebi

bwoebi Aug 9, 2015

playground (self-explaining?), php-gtk/ (same category than the pecl repos).

The other phpruntests/pftt2… no real idea what they even are …

Only maybe phd.git should be protected on master too.

bwoebi commented Aug 9, 2015

playground (self-explaining?), php-gtk/ (same category than the pecl repos).

The other phpruntests/pftt2… no real idea what they even are …

Only maybe phd.git should be protected on master too.

@johannes

This comment has been minimized.

Show comment
Hide comment
@johannes

johannes Aug 10, 2015

Member

I haven't reviewed this. But I'd suggest to change the error message. Something like

-deny("You are not allowed to overwrite commits on " . implode(', ', $restricted));
+deny("You are not allowed to overwrite commits (force push) on " . implode(', ', $restricted) . ". Please contact PHP Git admins");

(not sure we have a mailing list, should use systems@ or list specific names)

Member

johannes commented Aug 10, 2015

I haven't reviewed this. But I'd suggest to change the error message. Something like

-deny("You are not allowed to overwrite commits on " . implode(', ', $restricted));
+deny("You are not allowed to overwrite commits (force push) on " . implode(', ', $restricted) . ". Please contact PHP Git admins");

(not sure we have a mailing list, should use systems@ or list specific names)

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