git-p4: Jobs and skipSubmitEdit

git-p4: Jobs and skipSubmitEdit

[Michael Horowitz]

Hello,

I've written a git prepare-commit-msg hook to do what the Perforce
JobsView would essentially do, so I can include the jobs directly in
my git commit message, and then use git-p4.skipSubmitEdit=true, so I
can just push things into Perforce directly from git without ever
being prompted by Perforce.

Problem is that this doesn't work, because git-p4 tabs in the entire
commit message to put it in the "Description:" section of the Perforce
changelist, and my "Jobs:" ends up tabbed in, and it it required by
Perforce to be at the beginning of the line.  The submit ends up
failing, because "Jobs:" is required.  I am forced to turn off
skipSubmitEdit and edit the message to remove the tab from the "Jobs:"
line each commit.

Is there any option to make this work right, or does the git-p4 not
support this?

Thanks,

Mike

Re: git-p4: Jobs and skipSubmitEdit

[Pete Wyckoff]

mike@horowitz.name wrote on Fri, 22 Jun 2012 12:15 -0400:
> I've written a git prepare-commit-msg hook to do what the Perforce
> JobsView would essentially do, so I can include the jobs directly in
> my git commit message, and then use git-p4.skipSubmitEdit=true, so I
> can just push things into Perforce directly from git without ever
> being prompted by Perforce.
> 
> Problem is that this doesn't work, because git-p4 tabs in the entire
> commit message to put it in the "Description:" section of the Perforce
> changelist, and my "Jobs:" ends up tabbed in, and it it required by
> Perforce to be at the beginning of the line.  The submit ends up
> failing, because "Jobs:" is required.  I am forced to turn off
> skipSubmitEdit and edit the message to remove the tab from the "Jobs:"
> line each commit.
> 
> Is there any option to make this work right, or does the git-p4 not
> support this?

This is a great idea, and something I've thought would be useful
too.  There's no code to handle it currently, but it shouldn't be
too hard.

I'm imagining that special strings in the git commit are hoisted
up out of the description when building the p4 change message,
as you describe.  Are there any more than "Jobs:"?  Is it just
a single instance that might appear?

An easy way to do this is to change prepareLogMessage to alter
or add a Jobs line.  Unless we decide this needs to be more
general.

		-- Pete

Re: git-p4: Jobs and skipSubmitEdit

[Michael Horowitz]

Well, "Jobs:" is the only thing I use.  Everything else is filled in
automatically by git-p4.

Yeah, so "Jobs:" appears once on a line by itself, and then multiple
lines tabbed in of whatever the Job ids are, in our case they are JIRA
issue numbers.  So, it might look like:

Jobs:
    PROJECTNAME-123
    PROJECTNAME-456

I think that if someone is using git-p4 and they have "Jobs:" at the
beginning of a line by itself in a git commit message, then it is safe
to stop pre-pending tabs to the lines from there on.  It can also be
an option to turn this behavior on.

Mike


On Sun, Jun 24, 2012 at 4:24 PM, Pete Wyckoff <pw@padd.com> wrote:
> mike@horowitz.name wrote on Fri, 22 Jun 2012 12:15 -0400:
>> I've written a git prepare-commit-msg hook to do what the Perforce
>> JobsView would essentially do, so I can include the jobs directly in
>> my git commit message, and then use git-p4.skipSubmitEdit=true, so I
>> can just push things into Perforce directly from git without ever
>> being prompted by Perforce.
>>
>> Problem is that this doesn't work, because git-p4 tabs in the entire
>> commit message to put it in the "Description:" section of the Perforce
>> changelist, and my "Jobs:" ends up tabbed in, and it it required by
>> Perforce to be at the beginning of the line.  The submit ends up
>> failing, because "Jobs:" is required.  I am forced to turn off
>> skipSubmitEdit and edit the message to remove the tab from the "Jobs:"
>> line each commit.
>>
>> Is there any option to make this work right, or does the git-p4 not
>> support this?
>
> This is a great idea, and something I've thought would be useful
> too.  There's no code to handle it currently, but it shouldn't be
> too hard.
>
> I'm imagining that special strings in the git commit are hoisted
> up out of the description when building the p4 change message,
> as you describe.  Are there any more than "Jobs:"?  Is it just
> a single instance that might appear?
>
> An easy way to do this is to change prepareLogMessage to alter
> or add a Jobs line.  Unless we decide this needs to be more
> general.
>
>                -- Pete

Re: git-p4: Jobs and skipSubmitEdit

[Pete Wyckoff]

mike@horowitz.name wrote on Tue, 26 Jun 2012 01:07 -0400:
> Well, "Jobs:" is the only thing I use.  Everything else is filled in
> automatically by git-p4.
> 
> Yeah, so "Jobs:" appears once on a line by itself, and then multiple
> lines tabbed in of whatever the Job ids are, in our case they are JIRA
> issue numbers.  So, it might look like:
> 
> Jobs:
>     PROJECTNAME-123
>     PROJECTNAME-456
> 
> I think that if someone is using git-p4 and they have "Jobs:" at the
> beginning of a line by itself in a git commit message, then it is safe
> to stop pre-pending tabs to the lines from there on.  It can also be
> an option to turn this behavior on.

I agree that it is simpler to explain if Jobs: must come last.
For the option, I'd prefer not to add another one, and instead
hope that it's unlikely to find such a construct in a commit
message that does not want this feature to happen.

I coded it up already, but with a couple little differences.
Once I get a couple more of the series on which it depends mailed
out, I'll send it along for you to play with.

		-- Pete

> On Sun, Jun 24, 2012 at 4:24 PM, Pete Wyckoff <pw@padd.com> wrote:
> > mike@horowitz.name wrote on Fri, 22 Jun 2012 12:15 -0400:
> >> I've written a git prepare-commit-msg hook to do what the Perforce
> >> JobsView would essentially do, so I can include the jobs directly in
> >> my git commit message, and then use git-p4.skipSubmitEdit=true, so I
> >> can just push things into Perforce directly from git without ever
> >> being prompted by Perforce.
> >>
> >> Problem is that this doesn't work, because git-p4 tabs in the entire
> >> commit message to put it in the "Description:" section of the Perforce
> >> changelist, and my "Jobs:" ends up tabbed in, and it it required by
> >> Perforce to be at the beginning of the line.  The submit ends up
> >> failing, because "Jobs:" is required.  I am forced to turn off
> >> skipSubmitEdit and edit the message to remove the tab from the "Jobs:"
> >> line each commit.
> >>
> >> Is there any option to make this work right, or does the git-p4 not
> >> support this?
> >
> > This is a great idea, and something I've thought would be useful
> > too.  There's no code to handle it currently, but it shouldn't be
> > too hard.
> >
> > I'm imagining that special strings in the git commit are hoisted
> > up out of the description when building the p4 change message,
> > as you describe.  Are there any more than "Jobs:"?  Is it just
> > a single instance that might appear?
> >
> > An easy way to do this is to change prepareLogMessage to alter
> > or add a Jobs line.  Unless we decide this needs to be more
> > general.
> >
> >                -- Pete
> 

Re: git-p4: Jobs and skipSubmitEdit

[Michael Horowitz]

Pete,

I gave the patch a try, and it seems to work great.

Only problem I realized is that it means that the git commit message
and the p4 log message end up different, because to p4 the jobs lines
are special and get stripped out, but in git it just stays part of the
commit message.  So, when you do a p4 sync/rebase, the commits come
back without it.  This doesn't make much of difference now, because
the commit messages come back modified with the p4 path and changelist
number anyway, but if Luke tries to do that change to store the p4
path/changelist in notes instead, then we wouldn't want any
differences in the commit messages, because then the commits hashes
won't match again.

Not sure what the best thing to do here is, but I guess it doesn't
matter until this mythical notes thing happens.

Mike



On Tue, Jun 26, 2012 at 7:21 AM, Pete Wyckoff <pw@padd.com> wrote:
>
> mike@horowitz.name wrote on Tue, 26 Jun 2012 01:07 -0400:
> > Well, "Jobs:" is the only thing I use.  Everything else is filled in
> > automatically by git-p4.
> >
> > Yeah, so "Jobs:" appears once on a line by itself, and then multiple
> > lines tabbed in of whatever the Job ids are, in our case they are JIRA
> > issue numbers.  So, it might look like:
> >
> > Jobs:
> >     PROJECTNAME-123
> >     PROJECTNAME-456
> >
> > I think that if someone is using git-p4 and they have "Jobs:" at the
> > beginning of a line by itself in a git commit message, then it is safe
> > to stop pre-pending tabs to the lines from there on.  It can also be
> > an option to turn this behavior on.
>
> I agree that it is simpler to explain if Jobs: must come last.
> For the option, I'd prefer not to add another one, and instead
> hope that it's unlikely to find such a construct in a commit
> message that does not want this feature to happen.
>
> I coded it up already, but with a couple little differences.
> Once I get a couple more of the series on which it depends mailed
> out, I'll send it along for you to play with.
>
>                 -- Pete
>
> > On Sun, Jun 24, 2012 at 4:24 PM, Pete Wyckoff <pw@padd.com> wrote:
> > > mike@horowitz.name wrote on Fri, 22 Jun 2012 12:15 -0400:
> > >> I've written a git prepare-commit-msg hook to do what the Perforce
> > >> JobsView would essentially do, so I can include the jobs directly in
> > >> my git commit message, and then use git-p4.skipSubmitEdit=true, so I
> > >> can just push things into Perforce directly from git without ever
> > >> being prompted by Perforce.
> > >>
> > >> Problem is that this doesn't work, because git-p4 tabs in the entire
> > >> commit message to put it in the "Description:" section of the
> > >> Perforce
> > >> changelist, and my "Jobs:" ends up tabbed in, and it it required by
> > >> Perforce to be at the beginning of the line.  The submit ends up
> > >> failing, because "Jobs:" is required.  I am forced to turn off
> > >> skipSubmitEdit and edit the message to remove the tab from the
> > >> "Jobs:"
> > >> line each commit.
> > >>
> > >> Is there any option to make this work right, or does the git-p4 not
> > >> support this?
> > >
> > > This is a great idea, and something I've thought would be useful
> > > too.  There's no code to handle it currently, but it shouldn't be
> > > too hard.
> > >
> > > I'm imagining that special strings in the git commit are hoisted
> > > up out of the description when building the p4 change message,
> > > as you describe.  Are there any more than "Jobs:"?  Is it just
> > > a single instance that might appear?
> > >
> > > An easy way to do this is to change prepareLogMessage to alter
> > > or add a Jobs line.  Unless we decide this needs to be more
> > > general.
> > >
> > >                -- Pete
> >

Re: git-p4: Jobs and skipSubmitEdit

[Pete Wyckoff]

mike@horowitz.name wrote on Tue, 17 Jul 2012 04:49 -0400:
> I gave the patch a try, and it seems to work great.
> 
> Only problem I realized is that it means that the git commit message
> and the p4 log message end up different, because to p4 the jobs lines
> are special and get stripped out, but in git it just stays part of the
> commit message.  So, when you do a p4 sync/rebase, the commits come
> back without it.  This doesn't make much of difference now, because
> the commit messages come back modified with the p4 path and changelist
> number anyway, but if Luke tries to do that change to store the p4
> path/changelist in notes instead, then we wouldn't want any
> differences in the commit messages, because then the commits hashes
> won't match again.
> 
> Not sure what the best thing to do here is, but I guess it doesn't
> matter until this mythical notes thing happens.

Glad it works.

I didn't think about trying to make the commit messages exact.
That isn't required for, e.g. "git cherry", but might be nice
just to make git/p4 view of the changes more similar.

Okay if we wait and see how the notes stuff goes, like you
suggest.  Could be that Jobs ends up in a note too.

Thanks for testing.

		-- Pete