Page MenuHomePhabricator

Implement pre-commit Herald rules
Closed, ResolvedPublic

Description

See discussion in T4189.

I think I'm going to do this in two hook phases. The first phase is the "ref" phase and controls adding, changing, and removing tags, branches and bookmarks. The fields would be things like:

[New tags] [exist]
[Modified branches] [include] [master]
[Modified branches] [match regexp] [/whatever/]
[Deleted bookmarks] [include] [quack]
[Repository] [is] [rP]
[Pusher] [is] [alincoln]

The only action is "reject with message":

[Reject with message] ["Interns aren't allowed to touch master."]

This phase can't examine commit content, but can encode a bunch of rules like who can push to master, who can create or delete tags or branches, etc. This phase doesn't fire in Subversion, since there are no meaningful ref-like metadata artifacts.

The second phase is the "commit" phase and looks very similar to the existing commit hooks. It has fields like:

[Commit message] ...
[Affected files] ...
[Author] ...

...etc. It will have a few new special rules, like:

[Committer]
[Commit is new branch head?]
[Commit is merge?]
[Commit branches]

This permits all the content-based rules to fire.

I think this will cover the vast majority of cases. One issue is that putting all of this in Herald global rules means you may automatically have rules apply to repositories you create and be unable to edit them (e.g., if all users can create repositories but only admins can manage global rules). But this is good in the common case where you have multiple similar repositories, and I think we can wait and see if it's a problem in practice. Additional changes in the future (like tagging repositories with projects) should make it easier to write rules that don't step on anyone's toes, even for larger organizations.

Revisions and Commits

rPHU libphutil
D7762
rARC Arcanist
D7790
rP Phabricator
D7841
D7820
D7809
D7808
D7807
D7806
D7805
D7804
D7801
D7802
D7800
D7796
D7795
D7793
D7792
D7791
D7789
D7782
D7781
AuditedD7780
D7766
D7765
D7764
D7763
D7761
AuditedD7718
D7713
D7711
D7705

Event Timeline

epriestley claimed this task.
epriestley raised the priority of this task from to Normal.
epriestley updated the task description. (Show Details)
epriestley added a project: Diffusion.
epriestley added subscribers: epriestley, zeeg, davidressman and 2 others.
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.
epriestley edited this Maniphest Task.