Hi Genki, On Sun, 4 Feb 2018, Genki Sky wrote: > This option allows commits with empty commit messages to be rebased, > matching the same option in git-commit and git-cherry-pick. While empty > log messages are frowned upon, sometimes one finds them in older > repositories (e.g. translated from another VCS [0]), or have other > reasons for desiring them. The option is available in git-commit and > git-cherry-pick, so it is natural to make other git tools play nicely > with them. Adding this as an option allows the default to be "give the > user a chance to fix", while not interrupting the user's workflow > otherwise [1]. > > [0]: https://stackoverflow.com/q/8542304 > [1]: https://public-inbox.org/git/7vd33afqjh.fsf@xxxxxxxxxxxxxxxxxxxxxxxx/ > > To implement this, add a new --allow-empty-message flag. Then propagate > it to all calls of 'git commit', 'git cherry-pick', and 'git rebase--helper' > within the rebase scripts. > > Signed-off-by: Genki Sky <sky@xxxxxxxx> > --- > > Notes: > > Thanks for the initial feedback, here are the changes from [v1]: > - Made my commit message include the main motivations inline. > - Moved new tests to t/t3405-rebase-malformed.sh, which has the > relevant test description: "rebase should handle arbitrary git > message". > - Accordingly re-used existing test setup. > - Minimized tests to just one for git-rebase--interactive.sh and one > for git-rebase--merge.sh. First, one test per file keeps things > focused while getting most benefit (other code within same file is > likely to be noticed by modifiers). And, while git-rebase--am.sh > does have one cherry-pick, it is only for a special case with > --keep-empty. So git-rebase--am.sh otherwise doesn't have this > empty-message issue. > > In general, there was a concern of over-testing, and over-checking > implementation details. So, this time, I erred on the conservative > side. > > [v1]: https://public-inbox.org/git/9d0414b869f21f38b81f92ee0619fd76410cbcfc.1517552392.git.sky@xxxxxxxx/t/ Very nice. I looked over the patch (sorry, I have too little time to test this thoroughly, but then, it is the custom on this here mailing list to just review the patch as per the mail) and it looks very good to me. Junio, if you like, please add a "Reviewed-by:" line for me. Thanks! Johannes
