On Thu, Jan 22, 2026 at 02:49:59PM +0100, Gabriele Monaco wrote: > On Thu, 2026-01-22 at 10:10 -0300, Wander Lairson Costa wrote: > > On Wed, Jan 21, 2026 at 02:53:03PM -0300, Wander Lairson Costa wrote: > > > On Wed, Jan 21, 2026 at 02:57:02PM +0100, Gabriele Monaco wrote: > > > > Mmh, this is a bit fishy though. > > > > We the patch using the decorator seems fine, but highlights how this > > > > method > > > > isn't meant to be in Monitor if not all monitors use it.. > > > > Adding a stub here is just sweeping dust under the carpet. > > > > > > > > Here should probably keep the common part of fill_trace_h() in Monitor > > > > (e.g. > > > > replacing MODEL_NAME and other common things) and create specific > > > > implementations in dot2k and ltl2k for what is not common while calling > > > > the > > > > super() counterpart for the rest. > > > > > > > > Does it make sense to you? > > > > > > Yes, that is exactly my idea. Since the patch series were getting too > > > long and my brain too rot, I thought would be better addressing this in > > > a following up patch series. But I can work in the next version if you > > > are not ok with that approach. > > Good point, that can be a separate series so that we don't mix too many > things, > but I'd also separate the initial patch introducing the @not_implemented . > > > I gave more thought on this matter yesterday before bed. Maybe this > > isn't a issue on the design. Some methods on Monitor might just have a > > harmless default behavior. I look into it more closely for next the > > round. > > Well, I believe that if a bunch of methods from the parent class don't need to > be called and we have to create stubs just to avoid errors, those methods > probably shouldn't be there in the first place. > > That's particularly valid for the Container class, that won't ever need to > fill > tracepoints and other stuff. > > Why fill_tracepoint_args_skel() is not required by LTL is an implementation > detail, so that stub could even stay, in case future monitors are going to > need > the entire thing. > Though I still find it cleaner to move that away too until there's a need for > it > shared in Monitor. >
I didn't catch what is included in "that"... > What do you think? > I agreed. fill_tracepoint_args_skel() makes sense in the Monitor class. If a derived class doesn't need it, it is an implementation detail. > Thanks, > Gabriele >
