On Sun, Oct 23, 2016 at 01:41:25PM -0400, Aaron and Ashley Watson wrote: > > But what's going on here? Why did we bother running rev-parse earlier if > > we don't actually use the value of REV? > > > > You mentioned tweaking it to fix a broken test, and indeed, just using > > $REV here breaks a few tests in t3903. > > > > Offhand, I do not see anything wrong with pulling the non-option values > > out in the loop. But in that case I think the assignment of REV can just > > go away completely. > > > > The only reason for REV to remain is to preserve the error message seen with > the previous behavior. Perhaps it would be better to instead move the > assignment > of REV to the only place it is still used: the error message when multiple > arguments were detected. Ah, thanks, I missed that use. We suppress stderr, so we're literally just getting the set of revs there. But that should match what we have in ARGV anyway (after all, $ARGV is where we decided we had too many revs, and what we'll feed to rev-parse to get the sha1). So I wonder if: Too many revisions specified: $ARGV would be more appropriate (at which point you can probably just continue to call it $REV). > I'm not sure of the next steps in the process of submitting a patch. > Should I submit a new patch by replying to this email, or is using git > send-email to create a new mail thread better? Generally re-rolls of a patch are done as replies to the original. Gmail is bad about corrupting whitespace, though, so you can't just reply and paste the patch in there. You can use ask git-send-email to continue the thread, though: mid=1473378397-22453-1-git-send-email-watsona4@xxxxxxxxx git send-email --in-reply-to=$mid ...other options... You might also want to cc the people involved in the earlier discussion. The public-inbox archive gives a customized full send-email command for each message to make it easy. http://public-inbox.org/git/1473378397-22453-1-git-send-email-watsona4@xxxxxxxxx/ -Peff