vlsi commented on PR #6726: URL: https://github.com/apache/jmeter/pull/6726#issuecomment-5011640858
Follow-up to my previous comment, and please read this before you start reworking. I no longer think `PropertyBackedTestBean` is the right shape, so ignore step 2 of the path I sketched above. The marker names a storage strategy but its actual effect is "skip the field sync", and it doesn't extend `TestBean` despite the name. The better decomposition, suggested by looking at where `TestBean` is actually checked: it bundles two unrelated jobs. Five call sites use it to mean "generate my GUI from BeanInfo" (`TestBeanGUI`, `GuiPackage`, `MenuFactory`, `JMeterTreeNode`), and three use it to mean "copy my properties into my setters" (`TestBeanHelper`, `StandardJMeterEngine`). No call site needs both. Splitting it into two interfaces, with `TestBean` extending both, keeps every existing `instanceof` result identical for all 32 elements and every third-party plugin, while letting new elements opt into only the half they want. I've opened #6728 to work that out separately. It doesn't belong in a thread about `getFilename()`, and it needs input from other maintainers. So: nothing to rework here yet. This PR is blocked on a design decision that isn't yours to make, and I don't want you spending a third round on a shape we might change again. I'll ping you once there's a conclusion. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
