git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v8 4/8] notes: allow use of the "rewrite" terminology for merge strategies

From
Johan Herland <johan@herland.net>
Date
Aug 17, 2015, 12:54 UTC
Message-ID
<CALKQrgfLzWdRxC5saBXJ_-iKmVDfs+mBfDKKrSU2-tP7eO5+Zg@mail.gmail.com>
In-Reply-To
<1439801191-3026-5-git-send-email-jacob.e.keller@intel.com>
On Mon, Aug 17, 2015 at 10:46 AM, Jacob Keller <jacob.e.keller@intel.com> wrote:
Show 6 quoted lines
> From: Jacob Keller <jacob.keller@gmail.com>
>
> notes-merge.c already re-uses the same functions for the automatic merge
> strategies used by the rewrite functionality. Teach the -s/--strategy
> option how to interpret the equivalent rewrite terminology for
> consistency.

I'm somewhat negative to this patch. IMHO, adding the rewrite modes as merge strategy synonyms adds no benefit - only potential confusion - to the existing merge strategies. Words that have a sensible meaning in the context of rewrite, do not necessarily have the same sensible meaning in the context of merge (and vice versa). I'd rather have the rewrite code map ignore/overwrite/concatenate to ours/theirs/union, without teaching the notes-merge code about these words. Or maybe even drop this patch (and the next?) entirely, and let the future author (who implements notes rewrite in terms of notes merge) decide how to deal with this? By committing to these synonyms now, you might actually be making things harder for the future author: once the synonyms are part of the user-visible and documented interface, they cannot easily be removed/changed again.

...Johan
Show 6 quoted lines
> Add tests for the new synonyms.
>
> Teaching rewrite how to understand merge terminology is left for a
> following patch.
>
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
-- 
Johan Herland, <johan@herland.net>
www.herland.net
Previous: Jacob KellerNext: Jacob Keller
Message 6 of 14 in “implement notes.mergeStrategy option”
  1. 0/8 implement notes.mergeStrategy optionJacob Keller, Aug 17, 2015
  2. 1/8 notes: document cat_sort_uniq rewriteModeJacob Keller, Aug 17, 2015
  3. 2/8 notes: extract enum notes_merge_strategy to notes-utils.hJacob Keller, Aug 17, 2015
  4. 3/8 note: extract parse_notes_merge_strategy to notes-utilsJacob Keller, Aug 17, 2015
  5. 4/8 notes: allow use of the "rewrite" terminology for merge strategiesJacob Keller, Aug 17, 2015
  6. Johan HerlandAug 17, 2015
  7. Jacob KellerAug 17, 2015
  8. Junio C HamanoAug 17, 2015
  9. 5/8 notes: implement parse_combine_rewrite_fn using parse_notes_merge_strategyJacob Keller, Aug 17, 2015
  10. 6/8 notes: add tests for --commit/--abort/--strategy exclusivityJacob Keller, Aug 17, 2015
  11. 7/8 notes: add notes.mergeStrategy option to select default strategyJacob Keller, Aug 17, 2015
  12. 8/8 notes: teach git-notes about notes.<ref>.mergeStrategy optionJacob Keller, Aug 17, 2015
  13. Johan HerlandAug 17, 2015
  14. Jacob KellerAug 17, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.