Page MenuHomePhabricator

Make Change-Id optional
Closed, ResolvedPublic

Description

We use several other repositories/review systems than Gerrit (Github, Differential, Gitlab etc). It would be nice to use the same commit hook for all of them, but right now it requires the Gerrit-specific Change-Id footer. It would be nice if that was optional. (Not sure how to pull that off; it could check the URL of the upstream repository, but I doubt a post-commit hook has access to that. Maybe a command line option for the installer?)

Mentor(s): @jayvdb

Event Timeline

Tgr created this task.Nov 7 2017, 7:03 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 7 2017, 7:03 AM
jayvdb updated the task description. (Show Details)
jayvdb added a subscriber: jayvdb.
Paladox added a subscriber: Paladox.Jan 6 2018, 5:16 PM

Gerrit supports not having a change id, but dosen't support i think having a change reviewed without a change id due to it using that to update changes when you need to make further modifications.

Not including change id may cause other side effects too. Zuul uses the change id too (i think).

jayvdb added a comment.Jan 6 2018, 5:17 PM

@Paladox , this isnt about Gerrit. Please read first.

I did read it first. You want to use the same hook for GitHub, differential, gitlab and gerrit. But doint want to be forced to use the gerrit change id.

jayvdb added a comment.Jan 6 2018, 5:22 PM

There will be no impact on how this tool is installed in Gerrit.

Legoktm added a subscriber: Legoktm.Jan 6 2018, 6:22 PM

but I doubt a post-commit hook has access to that

I think you can shell out to git remote get-url origin probably.

rafidaslam added a subscriber: rafidaslam.

I'll work on this

Change 402775 had a related patch set uploaded (by Rafidaslam; owner: Rafid Aslam):
[integration/commit-message-validator@master] Make Change-Id optional in non-gerrit repositories

https://gerrit.wikimedia.org/r/402775

I think it'd be neat if this tool was using a library like GitPython instead of calling git from shell. We could get some advantages like increased test coverages (especially for this task which we need to make a test with repository that the remote upstream is from gerrit and another repository that the remote upstream is not from gerrit such as GitHub). Just a suggestion though.. :)

jayvdb added a comment.Jan 8 2018, 9:15 AM

Just do it.

Change 404156 had a related patch set uploaded (by Rafidaslam; owner: Rafid Aslam):
[integration/commit-message-validator@master] Add GitHubMessageValidator and add logic to switch the MessageValidator

https://gerrit.wikimedia.org/r/404156

Change 402775 abandoned by John Vandenberg:
Make Change-Id optional in non-gerrit repositories

Reason:
this is replaced with I862bf42561eb436258951947c8c4a1233a3416c5

https://gerrit.wikimedia.org/r/402775

Change 404156 merged by jenkins-bot:
[integration/commit-message-validator@master] Add GitHubMessageValidator and add logic to switch the MessageValidator

https://gerrit.wikimedia.org/r/404156

I've added a follow up GCI task to implement this in a Wikimedia project hosted on GitHub.
https://codein.withgoogle.com/dashboard/tasks/5025736417083392/

Legoktm closed this task as Resolved.May 19 2018, 6:24 PM

Closing as resolved since the necessary functionality has been implemented in commit-message-validator AFAIS.