On Thu, Jun 27, 2019 at 6:31 PM Emily Shaffer <emilyshaffer@xxxxxxxxxx> wrote: > On Thu, Jun 27, 2019 at 01:37:58AM -0400, Eric Sunshine wrote: > > Don't localize the die() message via _() here or in the preceding > > OBJ_COMMIT case. > > I'm a little surprised by that. Is it because die() is expected to only > be seen by the developer? Sorry, I was reading those as BUG(), not die(), and we don't localize BUG() text. But, why aren't those BUG()? Can those cases arise in practice? (Genuine question; I haven't familiarized myself with that code yet.) If they legitimately should be die(), then ignore my comment about not localizing them. > > The two die() messages are unnecessarily dissimilar. Try to unify them > > so that they read in the same way. > > I'm a little surprised by this too; it seems to me the root cause of > each would be different. In the former case, I'd guess that > traverse_commit_list()'s behavior changed, and in the latter case I'd > guess that a new object type was recently added to the model. Can you > help me understand the motivation for making the messages similar? Both causes you describe here sound like BUG() cases, not die(). If I'm understanding correctly, they could only trigger if someone made some breaking or behavior changing modifications within Git and failed to update all the code in the project impacted by the change. In other words, these can't be triggered by user input, hence they would be BUG()s indicating that a Git developer needs to fix the code. As for the messages themselves, I was referring to the grammatical dissimilarity of "unexpectedly" and "unexpected", and I also don't understand why one messages mentions walken_show_object() explicitly, whereas the other does not.
