| Message ID | 20190608191958.4593-1-rohit.ashiwal265@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Teach cherry-pick/revert to skip commits | expand |
On 06/09, Rohit Ashiwal wrote: > git am or rebase advice user to use git am --skip or git rebase --skip > to skip the commit that has become empty or has risen conflicts. OTOH, > cherry-pick advice user to use git reset HEAD which on the user’s part > is annoying and sometimes confusing. This patch series will bring > consistency between advices of these commands with introduction of > `--skip` flag to cherry-pick and revert. Thanks! I found a few minor nits that I missed in my off-list review on the PR. We should give others some time to comment now before you re-send it with the nits fixed (if you agree with them). One thing that would be good to add to future cover letters is a reminder of how this fits in to your GSoC project, where the overall end goal is to improve the consistency of sequencer commands, and this is one step along that way. > Rohit Ashiwal (3): > sequencer: add advice for revert > cherry-pick/revert: add --skip option > cherry-pick/revert: update hints > > Documentation/git-cherry-pick.txt | 4 +- > Documentation/git-revert.txt | 4 +- > Documentation/sequencer.txt | 4 ++ > builtin/commit.c | 13 ++++--- > builtin/revert.c | 5 +++ > sequencer.c | 26 ++++++++++++- > sequencer.h | 1 + > t/t3510-cherry-pick-sequence.sh | 63 +++++++++++++++++++++++++++++++ > 8 files changed, 108 insertions(+), 12 deletions(-) > > PR: https://github.com/r1walz/git/pull/1 > Reviewed-by: Elijah Newren <newren@gmail.com> > Reviewed-by: Thomas Gummerer <t.gummerer@gmail.com> Note that the 'Reviewed-by' footer is something that is "given" by the reviewers, and should only be added after they have explicitly given it for the patch/series in question. Instead this should probably mention that Elijah and me reviewed this off-list in the PR you linked to above. > -- > 2.21.0 >
Hey Thomas On Sun, Jun 9, 2019 at 2:32 PM Thomas Gummerer <t.gummerer@gmail.com> wrote: > > Thanks! I found a few minor nits that I missed in my off-list review > on the PR. We should give others some time to comment now before you > re-send it with the nits fixed (if you agree with them). Yes, I'll wait for some more reviews before sending another iteration. > One thing that would be good to add to future cover letters is a > reminder of how this fits in to your GSoC project, where the overall > end goal is to improve the consistency of sequencer commands, and this > is one step along that way. Sure, I'll keep this in mind and send some kind of summary in the next revision. > > > PR: https://github.com/r1walz/git/pull/1 > > Reviewed-by: Elijah Newren <newren@gmail.com> > > Reviewed-by: Thomas Gummerer <t.gummerer@gmail.com> > > Note that the 'Reviewed-by' footer is something that is "given" by the > reviewers, and should only be added after they have explicitly given > it for the patch/series in question. Instead this should probably > mention that Elijah and me reviewed this off-list in the PR you linked > to above. Oh! But I read here, SubmittingPatches[1] docs, that it's nice to give credits to those who helped you. Should we fix it in some way? I don't know. Anyway, that is beyond the scope of this patch. > > > -- > > 2.21.0 > > Thanks for the review Rohit [1]: https://github.com/git/git/blob/v2.22.0/Documentation/SubmittingPatches#L289-L291
On 06/09, Rohit Ashiwal wrote: > > > PR: https://github.com/r1walz/git/pull/1 > > > Reviewed-by: Elijah Newren <newren@gmail.com> > > > Reviewed-by: Thomas Gummerer <t.gummerer@gmail.com> > > > > Note that the 'Reviewed-by' footer is something that is "given" by the > > reviewers, and should only be added after they have explicitly given > > it for the patch/series in question. Instead this should probably > > mention that Elijah and me reviewed this off-list in the PR you linked > > to above. > > Oh! But I read here, SubmittingPatches[1] docs, that it's nice to give > credits to those who helped you. Should we fix it in some way? I don't > know. Anyway, that is beyond the scope of this patch. Right, thanks for thinking about credit here. The appropriate way to do that would be to add a Helped-by trailer to the individual commits, though especially for the first and last commit in the series we probably don't deserve that much credit ;) In past GSoC/Outreachy projects I've also seen the Mentored-by trailer. Other than Helped-by, which I've commonly used seen for this they do seem to be (more or less) documented in SubmittingPatches [2]. > Thanks for the review > Rohit > > [1]: https://github.com/git/git/blob/v2.22.0/Documentation/SubmittingPatches#L289-L291 [2]: https://github.com/git/git/blob/b697d92f56511e804b8ba20ccbe7bdc85dc66810/Documentation/SubmittingPatches#L353-L365