Require Change-Id

Antony Stubbs

unread,

Jun 12, 2010, 12:32:24 AM6/12/10

to Repo and Gerrit Discussion

Is there a way to require a change-id line in a commit (reject it if it's missing), like the requirement for signed-off-by ?

This would avoid trouble by accidentally leaving it out. (GitX doesn't trigger the git hook to execute, I think SmartGit doesn't either).

Cheers,
Tony.

Shawn Pearce

unread,

Jun 12, 2010, 3:13:34 AM6/12/10

to Antony Stubbs, Repo and Gerrit Discussion

On Fri, Jun 11, 2010 at 21:32, Antony Stubbs <antony...@gmail.com> wrote:
> Is there a way to require a change-id line in a commit (reject it if it's missing), like the requirement for signed-off-by ?

Not currently, no. :-(

> This would avoid trouble by accidentally leaving it out. (GitX doesn't trigger the git hook to execute, I think SmartGit doesn't either).

Bummer. But neither does EGit. So we're adding it natively to the
EGit plugin in version 0.9. :-)

Antony Stubbs

unread,

Jun 12, 2010, 3:23:44 AM6/12/10

to Shawn Pearce, Repo and Gerrit Discussion

Is it not as straight forward as adding another block similar to:

if (project.isUseSignedOffBy()) {
// If the project wants Signed-off-by / Acked-by lines, verify we
// have them for the blamable parties involved on this change.
//
boolean sboAuthor = false, sboCommitter = false, sboMe = false;
for (final FooterLine footer : c.getFooterLines()) {
if (footer.matches(FooterKey.SIGNED_OFF_BY)) {
final String e = footer.getEmailAddress();
if (e != null) {
sboAuthor |= author.getEmailAddress().equals(e);
sboCommitter |= committer.getEmailAddress().equals(e);
sboMe |= currentUser.getEmailAddresses().contains(e);
}
}
}
if (!sboAuthor && !sboCommitter && !sboMe && !ctl.canForgeCommitter()) {
reject(cmd, "not Signed-off-by author/committer/uploader");
return false;
}
}

return true;
}

S/signed-off-by/change-id/ ?

if (project.isChangeIdRequired()) {
for (final FooterLine footer : c.getFooterLines()) {
if (footer.matches(FooterKey.CHANGEI_ID)) {
return true;
}
}
reject(cmd, "Change-Id: missing");
return false;

Shawn Pearce

unread,

Jun 12, 2010, 10:28:52 PM6/12/10

to Antony Stubbs, Repo and Gerrit Discussion

Yea, that seems OK to me.

Antony Stubbs

unread,

Jun 12, 2010, 10:48:29 PM6/12/10

to Shawn Pearce, Repo and Gerrit Discussion

Are you going to patch it, or shall I submit something?

Regards,
Antony Stubbs

Shawn Pearce

unread,

Jun 13, 2010, 1:46:09 AM6/13/10

to Antony Stubbs, Repo and Gerrit Discussion

On Sat, Jun 12, 2010 at 19:48, Antony Stubbs <antony...@gmail.com> wrote:
> Are you going to patch it, or shall I submit something?

Its a lot easier for me to just review and click submit. :-)

Antony Stubbs

unread,

Jun 13, 2010, 2:04:19 AM6/13/10

to Shawn Pearce, Repo and Gerrit Discussion

On 13/06/2010, at 5:46 PM, Shawn Pearce <s...@google.com> wrote:

> On Sat, Jun 12, 2010 at 19:48, Antony Stubbs <antony...@gmail.com> wrote:
>> Are you going to patch it, or shall I submit something?
>
> Its a lot easier for me to just review and click submit. :-)

Fair enough! Do you thunk it will require a schema change?

Shawn Pearce

unread,

Jun 14, 2010, 10:04:01 AM6/14/10

to Antony Stubbs, Repo and Gerrit Discussion

On Sat, Jun 12, 2010 at 23:04, Antony Stubbs <antony...@gmail.com> wrote:
> On 13/06/2010, at 5:46 PM, Shawn Pearce <s...@google.com> wrote:
>
>> On Sat, Jun 12, 2010 at 19:48, Antony Stubbs <antony...@gmail.com> wrote:
>>> Are you going to patch it, or shall I submit something?
>>
>> Its a lot easier for me to just review and click submit. :-)
>
> Fair enough! Do you thunk it will require a schema change?

Unfortunately, yes. You added a property to the Project entity, and
need a new column in the database to store that "require change-id"
flag.

Jay Soffian

unread,

Jun 14, 2010, 2:29:55 PM6/14/10

to Antony Stubbs, Shawn Pearce, Repo and Gerrit Discussion

j.

> --
> To unsubscribe, email repo-discuss...@googlegroups.com
> More info at http://groups.google.com/group/repo-discuss?hl=en
>

Reply all

Reply to author

Forward