On 2021.08.05 04:47, Ævar Arnfjörð Bjarmason wrote: > > On Wed, Aug 04 2021, Taylor Blau wrote: > > > I didn't realize before that the unknown case really is dead code, so > > we'll never log "<unknown>". And since the mapping from protocol_version > > to string is identical for known values, we could probably do without > > it. > > > > And I don't much care either way. I think the benefit is really pretty > > slim, and arguably my code is just adding unnecessary overhead. So I'm > > happy to go with or without it, but I'd be rather sad to spend much more > > of our collective time discussing it. > > Yeah, I just think if we can be sure it's an integer *and* a valid > version when we log it, people writing future log summarizing code will > thank us, i.e. just 0, 1, 2, and in the future maybe 3, ..., but not -1 > or "<unknown>" or other values we'll trust die() etc. to handle. Sounds good, in V3 I will switch back to logging the enum value directly, and will make sure we don't log anything if the version is unknown. Thanks both for the discussion!
