On Mon, Oct 21, 2024 at 5:53 PM Taylor Blau <me@xxxxxxxxxxxx> wrote: > > On Mon, Oct 21, 2024 at 02:38:15PM +0000, Samuel Adekunle Abraham via GitGitGadget wrote: > > From: Abraham Samuel Adekunle <abrahamadekunle50@xxxxxxxxx> > > > > Notes can be added to a commit using: > > - "-m" to provide a message on the command line. > > - -C to copy a note from a blob object. > > - -F to read the note from a file. > > When these options are used, Git does not open an editor, > > it simply takes the content provided via these options and > > attaches it to the commit as a note. > > > > Improve flexibility to fine-tune the note before finalizing it > > by allowing the messages to be prefilled in the editor and edited > > after the messages have been provided through -[mF]. > > > > Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@xxxxxxxxx> > > Nicely described, this commit message is looking good. Let's take a look > at the patch below... > > > diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh > > index 99137fb2357..2827f592c66 100755 > > --- a/t/t3301-notes.sh > > +++ b/t/t3301-notes.sh > > @@ -1567,4 +1567,75 @@ test_expect_success 'empty notes do not invoke the editor' ' > > git notes remove HEAD > > ' > > > > +test_expect_success 'git notes add with -m/-F invokes editor with -e' ' > > + test_commit 19th && > > + MSG="Edited notes message" git notes add -m "Initial notes message" -e && > > + echo "Edited notes message" >expect && > > Very nice use of the fake_editor script here. > > It is a little cumbersome to repeat the same message in MSG= and when > populating the 'expect' file. Perhaps instead this could be written as: > > echo "edited notes message" >expect && > MSG="$(cat expect)" git notes -add -m "initial" -e Concise! :-). Thank you very much Taylor. > > > + git notes show >actual && > > + test_cmp expect actual && > > + git notes remove HEAD && > > + > > + # Add a note using -F and edit it > > + echo "Note from file" >note_file && > > + MSG="Edited note from file" git notes add -F note_file -e && > > + echo "Edited note from file" >expect && > > Same "note" here. ;-). > > > + git notes show >actual && > > + test_cmp expect actual > > +' > > + > > +test_expect_success 'git notes append with -m/-F invokes the editor with -e' ' > > + test_commit 20th && > > + git notes add -m "Initial note message" && > > + MSG="Appended edited note message" git notes append -m "New appended note" -e && > > It's fine to use shorter values for -m and $MSG here. I think "appended" > and "edited" would be fine for each, respectively. > Okay. Noted > Besides applying those suggestions throughout the patch's new tests > (including the ones that I didn't explicitly comment on here), I think > that this should be looking good after another round. Thanks for working > on it. Thank you very much, Abraham Samuel > > Thanks, > Taylor
