Wireshark mailing list archives
Re: Why are authors never Cc'ed before their code is changed?
From: Sylvain Munaut <246tnt () gmail com>
Date: Sun, 19 Aug 2012 22:04:48 +0200
Hi Pascal,
For me not all changes have the same potential of breakage: a change in an API used by an external tool can easily lead to a complete incompatibility (GSMTAP format unapproved changes for example) while a filter rename or a wrong subtree used to display a field (GMR-1 breakages seen lately) are "just" an annoyance (unless you have an external tool configuring filters by command line?).
To me, anything that impacts input / output should be treated with special attention. This breaks documentation for example, because all info becomes outdated ...
That said, applying general rules (like filter names) to the whole code base once defined should have precedence over a single developer feelings.
Sure, I'm all for consistency but sometimes rules need to be revised for changing environment. Now I will leave this particular discussion for the other thread were I explained my arguments.
I can see different type of commits: [...snip...] - general code cleaning spreading across several files. Usually it should not impact the dissector usability while complying with the coding style. Getting in touch with any contributor simply because a given Wireshark internal API gets deprecated and replaced by another one is not doable from my point of vue. When the changes are related to warnings removal (if I remember correctly it was the root cause of one of the two regressions seen with GMR-1 dissector) author could be contacted also. To avoid this kind of problems, an author can also check by himself the Clang / MSVC Code Analysis warnings for his own dissectors as they are freely available on the buildbot pages ;)
My opinion is that I'd like to be CC'ed for every change. Obviously that needs to be automatic in someway and I guess I can setup a cron myself for that to check the tree periodically ... I mean even API changes I'm interested, even just because I always have "newer / experimental" version of the dissector in my tree and they may need updating as well. I agree that for API changes there is not much point in asking the author his opinion. But if I take the last 3 changes to the gmr dissector as example: - PD tree warning fixes - Rename of RR to CCCH (granted you fixed it yourself, but still I think it shouldn't have happened in the first place) - Filter changes Those 3 changed the way the dissector worked. The code before and after was not equivalent (not like replacing a 0 by a ENC_BIG_ENDIAN for example). And each time I think the problem happened because the person writing the patch had no understanding of GMR-1 itself. For instance, part of the code you can't possibly understand why it's been done a given way just by the code ... simply because the dissector is not complete and it takes good knowledge of GMR-1 to see the logic in it, to see how it will expand in the future). And I've seen this kind of thing happen to other dissectors in the past. For a while the display of the GSM dissected packet was broken (IE name repeated twice), seems to be fixed now. For those kind of changes I think consulting the author would be nice. Of course you're right that this is not a trivial process to implement. Personally I think that having a standardized way to signal (beyond just the copyright) the "maintainer" of a module would be a start, so that you have a clear point of contact (and if the original author doesn't want to maintain, just don't put his name ...). As for the timeout I think it can be pretty short. All the guy has to do if he doesn't have the time to do a full review is respond quickly with "Hold on, I'll review this during the week". This would also help with reporting bugs. I mean right now if someone post a decoding bug for the GMR proto on the wireshark and doesn't cc me explicitely I'm not sure I'll ever see the question ... (I'm subscribed to the ML ... but I'm subscribed to so many ML ...) Just my 2ct. Cheers, Sylvain ___________________________________________________________________________ Sent via: Wireshark-dev mailing list <wireshark-dev () wireshark org> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
Current thread:
- Why are authors never Cc'ed before their code is changed? Harald Welte (Aug 19)
- Re: Why are authors never Cc'ed before their code is changed? Evan Huus (Aug 19)
- Re: Why are authors never Cc'ed before their code is changed? Pascal Quantin (Aug 19)
- Re: Why are authors never Cc'ed before their code is changed? Sylvain Munaut (Aug 19)
- Re: [Wireshark-dev] Why are authors never Cc'ed before their code is changed? Christopher Maynard (Aug 19)
- Re: Why are authors never Cc'ed before their code is changed? Lori Jakab (Aug 20)
- Re: Why are authors never Cc'ed before their code is changed? Pascal Quantin (Aug 23)
- Re: Why are authors never Cc'ed before their code is changed? Lori Jakab (Aug 23)
- Re: Why are authors never Cc'ed before their code is changed? Evan Huus (Aug 23)