Oswald Buddenhagen <oswald.buddenhagen@xxxxxx> writes: > On Fri, Apr 28, 2023 at 12:01:02PM -0700, Junio C Hamano wrote: >>Oswald Buddenhagen <oswald.buddenhagen@xxxxxx> writes: >> >>> This makes sure that we get a properly translated message rather than >>> inserting the command (which we failed to translate) into a generic >>> fallback message. >> >>Hmph, can this be accompanied with a change to add a test to an >>existing test script to demonstrate that the function can be called >>with me set to "rebase" and results in a generic message? >> > i suppose it could, but see next paragraph. > >>> We now also BUG() out when encountering an unexpected command. >> >>This needs to be reviewed by somebody who is more familiar with the >>rebase/chrry-pick/revert/sequencer codepaths so that they can give a >>definitive "good--I know that we never call this function with any >>other value in 'me'" and that person would not be me. >> > assuming we care only about in-tree code, i'm just about as confident > about this as one can reasonably be - because i grepped through the > code, recursively looking for entry points. there are several calls > via die_resolve_conflict() which have hard-coded `me`s (none of which > is rebase), and two from the sequencer, where `me` comes from > action_name(), which in turn returns one of three hard-coded strings > (one of which is rebase). the latter is also kinda the test case, > because it is obvious that this will be actually invoked when the > situation occurs. it's probably also how i actually ran into the > problem in the first place (i surely wasn't *looking* for it ...). > >>> Arguably, it would be cleaner to pass the command as an enum in the >>> first place ... >> >>True, but that can be left to a different topic, I would think. >> > yes, otherwise i'd have already done it. ^^ > i can make it more explicit if you prefer that. > > if you agree with the reasoning, i'll prepare an update to the commit > message and leave the patch as-is. Yeah, how you arrived the conclusion that just covering "rebase" in addition to what the code already covers would make the if/elseif cascade complete is a very helpful addition to the log message.
