Let's decide in a techboard meeting whether traces are mandatory or not.
01/09/2023 04:32, Jerin Jacob: > On Thu, Aug 31, 2023 at 12:08 AM Morten Brørup <m...@smartsharesystems.com> > wrote: > > > > > From: Thomas Monjalon [mailto:tho...@monjalon.net] > > > Sent: Wednesday, 30 August 2023 18.24 > > > > > > 21/08/2023 16:46, Morten Brørup: > > > > > From: Ankur Dwivedi [mailto:adwiv...@marvell.com] > > > > > Sent: Monday, 21 August 2023 15.54 > > > > > > > In general, I wonder how much the check is useful compared to the > > > complexity. > > > > > > > > > > The bigger question is: Do we really want to change tracepoints in > > > functions from opt-in to opt-out? > > > > > > Yes that's the question: should traces be mandatory in some libs? > > > > > > > In my opinion, opt-in for trace is more appropriate. > > > > > > There was some work to add traces everywhere in few libs, so why not > > > maintaining this state? > > > > I still think it's a silly and burdening requirement, but I'm not against > > it for the libs where it is already a de facto standard. > > > > <irony> > > I can imagine similar requirements regarding logging, telemetry and dump > > functions being imposed on select libs. > > > > Perhaps we should also require that new libs implement all four types of > > instrumentation, to ensure that only high quality (i.e. fully instrumented) > > libs are accepted into DPDK. > > IMO, There is a lot of effort to put trace points to existing > libraries, without these check, soon the disparity shows up when > someone adds new APIs to library and forget > to add trace points. > > It is pretty easy to add trace point and there are a lot of > references, so I don't think there is burden for a developer > comparing to the effort of adding a new API. > > Most of the time, contributors forgets to add trace point because > there is no automatic way to find it. > Also, additional git commits are needs if some decide to add it later. > > No strong opinion, If not find not useful, we could mark this patch is > not appliable. Keeping the patch is limbo state is the issue. It is > already reached to v5. > So please decide one way or another. > > > > > > </irony> > > > > > I don't really like adding a skip list as one more burden for future > > > authors. > > > > If the warning omitted from checkpatches refers to the skip list and its > > location, it is relatively easy for developers to manage. > > > > And reviewers will notice if new functions have been added to the skip > > list, indicating that trace has been omitted. So there are also advantages > > to the skip list.