bamaer commented on PR #8408: URL: https://github.com/apache/hop/pull/8408#issuecomment-5702647143
@mattcasters thanks for the thorough review. All seven items are addressed in the latest commit. ### Bugs **1. Source-transform contract.** Added the three overrides mirroring `RowGeneratorMeta`: `getTransformIOMeta()` returning `new TransformIOMeta(false, true, false, false, false, false)`, `consumesMainInput()` false, `canStartWithoutInput()` true. `check()` now raises an error when `prev` is non-empty and an OK remark otherwise, with two new i18n keys. `PluginCatalog` is added to `never-consume-at-default-plugins.txt`, and `TransformInputConsumptionInventoryTest` passes. Its assertion is symmetric, so the entry is now required rather than optional. **2. Dialog.** Moved to the annotation framework: `@GuiPlugin` on the Meta with `@GuiWidgetElement` on all four options (three `CHECKBOX`, one `COMBO`), `groupType = GuiWidgetGroupType.BOXES`. The dialog is now `createShell` + `buildButtonBar` + `GuiCompositeWidgets.addScrolledComposite(...)`, down from 136 lines to 80. `changed = input.hasChanged()` is captured on open, `input.setChanged(changed)` on cancel, and `input.setChanged()` on OK. Your point about the buttons was right. Verified in the GUI, and both the missing left-hand labels and the button placement are resolved by the framework. **3. Nested descent.** Dropped the `org.apache.hop` prefix. Guards are now null/enum/primitive/array plus a `java.` / `javax.` / `jakarta.` short-circuit, with descent driven purely off "carries `@HopMetadataProperty` fields". It also consults `annotation.listItemClass()` before the generic parameter, matching how `XmlMetadataUtil` resolves raw lists. Locked with a fixture class in a real `io.github.thirdparty` package plus a raw-list fixture, so the third-party claim is now tested rather than asserted. ### Suggestions **4. `groupKey`.** Added as its own `property_group_key` column, with `group_key` in the `PER_PLUGIN` JSON. A separate column rather than encoding into an existing field, matching how `metastructure` gives each attribute its own column. Properties marked `isExcludedFromSerialization` are now skipped, and the docs state explicitly that `property_group` is nesting and not the serialization wrapper. One part not done: scanning getter-method `@HopMetadataProperty` the way `XmlMetadataUtil` does. No in-tree plugin currently annotates a getter, so it is a theoretical gap today, and it is a larger change than the rest. I would rather handle it as a follow-up than bundle it here. Happy to open an issue if you think it should land before this merges. **5.** `documentationUrl` is now `/pipeline/transforms/plugincatalog.html`, so `Const.getDocUrl()` prefixes the versioned base. **6.** `catch (Throwable)` is now `catch (Exception | LinkageError)` in both places, so fatal JVM errors propagate. **7.** Removed the comments that narrate control flow in `PluginCatalog`, `PluginCatalogReader` and `PluginCatalogData`. Kept the one explaining why a plugin with no properties still emits a row, which was the case you flagged as worth keeping. ### Verification 13 unit tests (up from 9, with the four new ones covering items 3 and 4), 0 checkstyle violations, spotless and rat clean, integration test `0113-plugin-catalog` passing, and a full build of 319 modules with no failures or skips. -- 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]
