On Thu, Jun 27, 2019 at 08:48:31PM -0400, Eric Sunshine wrote: > 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. Hmmm. Yeah, I'll switch them to BUG() - I think there are other instances of die() in the example and it'd be good to describe yet another way of reporting behavior. > > > > 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. I see - ok, I have reworded. Thanks! - Emily
