commit-log-editor should allow git commit --amend to regenerate the commit log based on the modifed ChangeLog. It would be especially useful after being reviewed, and when editing the ChangeLogs of an already existing commit.
Created attachment 33465 [details] 2009-07-24 Pierre d'Herbemont <pdherbemont@apple.com> Reviewed by NOBODY (OOPS!). commit-log-editor should allow git commit --amend to regenerate the commit log based on the modifed ChangeLog https://bugs.webkit.org/show_bug.cgi?id=27664 * Scripts/commit-log-editor: Add --allow-commit-log-suppression option. The user is asked if he wants to suppress previous ChangeLog and regenerate it, if this option is enabled. --- 2 files changed, 32 insertions(+), 2 deletions(-)
Comment on attachment 33465 [details] 2009-07-24 Pierre d'Herbemont <pdherbemont@apple.com> > + * Scripts/commit-log-editor: Add --allow-commit-log-suppression option. > + The user is asked if he wants to suppress previous ChangeLog and regenerate it, > + if this option is enabled. I think "--allow-commit-log-suppression" should have a better name. Maybe --regenerate-log? > +my $previousLogMessage = ""; [...] > - $logContents .= $_; > + if(!isGit() || (/^#/)) { > + $logContents .= $_; > + } else { > + $previousLogMessage .= $_; > + } > $existingLog = isGit() && !(/^#/ || /^\s*$/) unless $existingLog; The $previousLogMessage contents don't seem to be used anywhere else. What happens if you want to use the previousLogMessage? Should $previousLogMessage be assigned (or appended) to $logContents? > +my $keepExistingLog = 1; > +if ($allowPreviousCommitLogSuppression && $existingLog) { > + print "Existing log message detected, enter 'd' to delete, and regenerate log message from ChangeLogs. Press Return to continue.\n"; > + my $key = getc(STDIN); > + $keepExistingLog = 0 if ($key eq "d"); > +} My reading of the code indicates that *any* key besides 'd' will cause the existing log to be kept. I'd change the message to something like: print "Existing log message detected. Use 'r' to regenerate from ChangeLogs, or any other key to keep the existing message.\n"; r- to address the above issues.
(In reply to comment #2) > (From update of attachment 33465 [details]) > > + * Scripts/commit-log-editor: Add --allow-commit-log-suppression option. > > + The user is asked if he wants to suppress previous ChangeLog and regenerate it, > > + if this option is enabled. > > I think "--allow-commit-log-suppression" should have a better name. Maybe > --regenerate-log? Seems like a better choice. > > > +my $previousLogMessage = ""; > [...] > > - $logContents .= $_; > > + if(!isGit() || (/^#/)) { > > + $logContents .= $_; > > + } else { > > + $previousLogMessage .= $_; > > + } > > $existingLog = isGit() && !(/^#/ || /^\s*$/) unless $existingLog; > > The $previousLogMessage contents don't seem to be used anywhere else. What > happens if you want to use the previousLogMessage? Should $previousLogMessage > be assigned (or appended) to $logContents? Yes, it's not being used here. It's here just because it's easier to read IMO. At second thought, it mades little sense, and a comment will probably be better. We could add it back as a comment in the new changelog. I didn't like this very much because it most of the time clutters the view of the modified files etc. > > > +my $keepExistingLog = 1; > > +if ($allowPreviousCommitLogSuppression && $existingLog) { > > + print "Existing log message detected, enter 'd' to delete, and regenerate log message from ChangeLogs. Press Return to continue.\n"; > > + my $key = getc(STDIN); > > + $keepExistingLog = 0 if ($key eq "d"); > > +} > > My reading of the code indicates that *any* key besides 'd' will cause the > existing log to be kept. I'd change the message to something like: > > print "Existing log message detected. Use 'r' to regenerate from ChangeLogs, > or any other key to keep the existing message.\n"; Actually, it's waiting for a return. I'll switch so that it uses Term::ReadKey package and actually just wait for one key. And change the comment. Thanks for the review! Pierre.
Created attachment 33585 [details] 2009-07-24 Pierre d'Herbemont <pdherbemont@apple.com> Reviewed by NOBODY (OOPS!). commit-log-editor should allow git commit --amend to regenerate the commit log based on the modifed ChangeLog https://bugs.webkit.org/show_bug.cgi?id=27664 * Scripts/commit-log-editor: Add --regenerate-log option. The user is asked if he wants to suppress previous ChangeLog and regenerate it, if this option is enabled. --- 2 files changed, 43 insertions(+), 2 deletions(-)
I have updated the patch so that: - Review comment are integrated - We implement usage() as a subroutine - Just one key is read - If there is no ChangeLog file modified we don't ask the user if he wants to get rid of its current log and regenerate the logs
Comment on attachment 33585 [details] 2009-07-24 Pierre d'Herbemont <pdherbemont@apple.com> > +sub usage > +{ > + print "Usage: [--help] [--regenerate-log] <log file>\n"; > + exit 0; > +} The usage() method should always exit with a non-zero status. I suggest "exit 1;". r=me with the above change. Thanks!
https://trac.webkit.org/changeset/46453