Checkin Test Script Feature Request: Add Extra String to Push Commit Message

[csiefer2]

It might be useful to allow the checkin test script to add an extra string to the push commit message.

For instance, this might be very nice in the case of manual code reviews and/or pair programming.

[bartlettroscoe]

@csiefer2,

Can you give some more concrete use cases for what you would pass into an argument like this?

Could you accomplish this with just git commit --amend before running the checkin-test.py script? What would be the advantage of passing this info in through an argument to checkin-test.py instead of using git commit --amend?

For some background, the reason the checkin-test.py script amends the top commit log by default is that is writes in info that only the checkin-test.py script has (i.e. what builds were done, how many tests were run and passed, if a forced push was done, etc.) right at the moment before it pushes. It is critical that such info be written in a uniform way (see Using git bisect with checkin-test.py workflows) and that it be written quickly so that the push can occur immediately (for the best chance of satisfying the "additive test assumption of branches").

Anyway, this is not a terribly hard feature to add but strong automated tests would need to be added to make sure it works correctly. I just want to make sure that adding this argument is the best way to accomplish the end goal.

[csiefer2]

I see this as a means add a comment that has some sort of "final" description of the feature my string of commits is adding to the code. Maybe with a note that says, "This code was reviewed / pair programmed with person X."

One could do git commit --amend before running the checkin-test.py script. However, if checkin test script failed because the code is broken, you'd want to remove that "final" message with another amend, checkin the fix and amend things again.

I agree that this would have to be written quickly, which implies that this option be invoked with either a string on the command line of the checkin script or via a file, something like,

./checkin-test.py --add-message "This code was reviewed by person X"

or: ./checkin-test.py --add-message-file myfinalcommitfile.txt

[bartlettroscoe]

Okay, if the goal is to make sure and always amend the top commit message before the final push, the same commit that gets amended before the final push, then I can see the need for this option and not just using git commit --amend manually. However, if the push fails and you don't amend, then you may need to create new commits to fix the problem and those might need to be reviewed as well. In that case having the checkin-test.py script mark a "manual review" is less useful. But having it mark it with "pair programming" could be useful (since you would not want to mark every commit with that). But how many people use pair programming? There are some arguments that pair programming most tasks is not a good usage of people's time. See:

• http://namcookanalytics.com/high-costs-and-negative-value-of-pair-programming/

[csiefer2]

I suspect that folks don't pair program all of the time. I don't. But it can be very useful for difficult to find bugs.