On Fri, Feb 23, 2018 at 1:30 PM, Brandon Williams <bmwill@xxxxxxxxxx> wrote: > On 02/22, Stefan Beller wrote: >> > +static enum protocol_version discover_version(struct packet_reader *reader) >> > +{ >> ... >> > + >> > + /* Maybe process capabilities here, at least for v2 */ >> > + switch (version) { >> > + case protocol_v1: >> > + /* Read the peeked version line */ >> > + packet_reader_read(reader); >> > + break; >> > + case protocol_v0: >> > + break; >> > + case protocol_unknown_version: >> > + die("unknown protocol version: '%s'\n", reader->line); >> >> The following patches introduce more of the switch(version) cases. >> And there it actually is a >> BUG("protocol version unknown? should have been set in discover_version") >> but here it is a mere >> die (_("The server uses a different protocol version than we can >> speak: %s\n"), >> reader->line); >> so I would think here it is reasonable to add _(translation). > > This should be a BUG as it shouldn't ever be unknown at this point. And > I'll also drop that comment. Huh? Then I miss-understood the flow of code. When the server announces its answer is version 42, but the client cannot handle it, which die call is responsible for reporting it to the user? (That is technically a BUG on the server side, as we probably never asked for v42, so I would not want to print BUG locally at the client?)