> On May 4, 2020, at 3:46 PM, Andrew Yourtchenko <ayour...@gmail.com> wrote: > More inline: >> On 4 May 2020, at 19:20, Paul Vinciguerra <pvi...@vinciconsulting.com> wrote: >> >> I agree that the change I submitted is a monster. Honestly, there are 2-5 >> git-friendly changes that could be refactored out of it. Its written and >> used by me, but it's not worth investing the time to make mergeable if there >> is no desire for it. > > That’s why it’s handy to discuss the big changes that touch the APIs upfront, > with the end users. Just last week I saw a mail on how the past year there > were so many API changes, which make life harder. VPP’s reason for existence > (at least in my view) is its users. > > This change reshuffles the messages and changes semantics for a bunch of > calls for the sole purpose of architectural purity. Purity is beautiful but > the users deserve a heads up at least. > >> I only submitted it at all because I had mentioned last week to someone >> that I had essentially stopped contributing and I was asked if I could be >> enticed to start re-submitting code. There is no intent on my part to >> subvert the release. >> >> Everyone complains about the tests. I was "asked" last year to clean up the >> tests. It's not that code hasn't been written to clean it up. The reality >> is that code becomes a monster when simple refactorings aren't merged. > > The tests need proper debugging first, *before* doing the refactorings. The > tests still can not run with high concurrent number of jobs. (N~48). > The tests still fail intermittently. > > This is what folks mean when when complaining. Rearranging the calling > convention doesn’t help this in the slightest, just adds unnecessary churn. > >> >> If you all tell me you want the changes, I'm glad to clean it up. > > That change contains multiple pieces which you yourself described. > > - dump by sw_if_index ? Sure. But again, not across the entire code base but > component by component (if there are many). Reviewed by the maintainer of the > component.
The fact that this might be a bit painful to do in parts is a nice feedback to the pain that the change causes downstream. :) I like consistent APIs as much as the next person, maybe more so, but non-backward compatible changes in order to tidy up the API cause pain and maybe aren't so great. That said, adding filter functionality is good, so if there wasn't the ability to filter on sw_if_index before, I'd vote for it. Thanks, Chris. > > - changing the message structure ? Again, if there are multiple components, > it should not be hard making small changes that would be trivial to review > (though before that, like i said, it would be nice to prove the users and > subsystem maintainers what they think of the changes). Also how do we ensure > the “old” inconsistency doesn’t resurrect in the new messages ? If there is > no automated check against that, this is just adding churn. > > - internal refactorings that have zero user impact. Why not. But again, if > you are going across components, each commit should be within a component, > and reviewed/committed by the maintainer of the component. > > —a > >> >> If, in the meantime, someone wants to +2 Neale's change [2], that would be >> great! >> >> [0] https://gerrit.fd.io/r/c/vpp/+/26833 >> [1] https://lists.fd.io/g/csit-dev/message/4009 >> [2] https://gerrit.fd.io/r/c/vpp/+/26820/4 >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#16236): https://lists.fd.io/g/vpp-dev/message/16236 Mute This Topic: https://lists.fd.io/mt/73980355/21656 Group Owner: vpp-dev+ow...@lists.fd.io Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
