bamaer commented on PR #8647: URL: https://github.com/apache/hop/pull/8647#issuecomment-5866864625
**Big picture: this is a real improvement.** The old single `hop-sdk.adoc` was thin, and the modular split plus per-plugin-type pages is the structure this section needed. Keeping `hop-sdk.adoc` as a redirect rather than deleting it is the right call. **Checked and holding up:** all 21 new files carry the ASF header; every internal `xref:` and `manual::` reference resolves (also in the merged-with-main tree); the `xref:sdk/...` style matches how `database/` and `architecture/` pages reference siblings; all 15 plugin annotation names are correct, as are `ConfigPlugin.CATEGORY_*` (all 11), `HopExtensionPoint`, `IExtensionPoint.callExtensionPoint`, the `WorkflowMeta` constructors, `WorkflowHopMeta.setUnconditional/setEvaluation`, all four metadata provider constructors, `MetaSelectionLine`, `GuiWidgetElement` attributes, and both engine factory signatures. The `@HopMetadata` guidance (kebab-case `key` + `legacyKeys` on rename) matches project convention exactly. **One thing to settle first.** `annotation-derived-widgets.adoc` landed on main on 27 Sep and is now the canonical description of the annotated-widget dialog pattern. `sdk/plugins/transforms.adoc`, `actions.adoc`, `metadata.adoc` and `gui.adoc` each restate that pattern, and their version doesn't compile (details below). Rather than repair four copies, I'd cut those sections down to a sentence plus `xref:annotation-derived-widgets.adoc[]`. Related: after merge the sidebar carries two parallel plugin-development trees — "Plugin development" (with `plugin-types/*`, `database/*`, `metadata-plugins.adoc`, `value-types.adoc`, `gui-plugins-toolbars.adoc`, `annotation-derived-widgets.adoc`) and "Plugin Development Guide" under Embedding, and none of the 16 new pages link to any of the older ones. Cross-linking now would stop them drifting; merging the trees can wait. **Should fix — samples appear written from memory rather than from the tree:** | Page | Issue | Fix | |---|---|---| | `sdk/index.adoc` | `hop-ui-swt` artifact doesn't exist | `hop-ui` | | `sdk/index.adoc` | `org.apache.hop.ui.core.gui.HopGuiEnvironment` | `org.apache.hop.ui.hopgui.HopGuiEnvironment` | | `sdk/pipelines.adoc` | both `new PipelineMeta(...)` calls pass a 4th boolean | actual: `(String\|InputStream, IHopMetadataProvider, IVariables)` | | `sdk/pipelines.adoc` | `rowProducer.setFinished()` | `finished()` | | `sdk/metadata.adoc` | `serializer.findAll()` | `loadAll()` | | `sdk/metadata.adoc` | `SerializableMetadataProvider` import | `org.apache.hop.core.metadata.SerializableMetadataProvider` | | `sdk/workflows.adoc` | factory wants `ILoggingObject`; sample passes `new LogChannel(...)`, which implements only `ILogChannel` | pass a `LoggingObject` | | `plugins/commands.adoc` | `org.apache.hop.command.*` | `org.apache.hop.hop.plugin.*` | | `plugins/file-types.adoc` | `HopFileTypePlugin` import; `hasCapability(HopFileTypeCapabilities)` with a switch | `org.apache.hop.ui.hopgui.file.HopFileTypePlugin`; `hasCapability(String)` (class is `FileTypeCapabilities` under `...explorer.file.capabilities`) | | `plugins/configuration.adoc` | `handleOption(ILogChannel, IHopMetadataProvider, IVariables)` | `IHasHopMetadataProvider`, `throws HopException` | | `plugins/servlets.adoc` | `javax.servlet.*` | Hop is on `jakarta.servlet.*` | | `plugins/variable-resolvers.adoc` | `resolver.type.VariableResolver` import; `init() throws HopException` | `...resolver.VariableResolver`; `IVariableResolver.init()` declares no checked exception | Two worth more than a line: **`sdk/metadata.adoc` — `MultiMetadataProvider` precedence is inverted.** `MultiMetadataSerializer.save()` picks `providers.get(providers.size() - 1)`, and `load()` (and the other lookups) iterate backwards from the end, so the **last** provider is the write target and wins on read. The sample labels the first entry "Primary provider: saves default here" — following it writes metadata to the wrong place. **The dialog recipe in `plugins/transforms.adoc` and `plugins/actions.adoc` won't compile** — four separate problems: `ButtonBarBuilder.build()` returns `void`, so `Button[] buttons = buildButtonBar()...build()` and `buttons[0]` don't work; `GuiCompositeWidgets.addScrolledComposite(...)` is static and *returns* the widgets object, but the sample builds a separate `new GuiCompositeWidgets(variables)` and discards the return, so `ok()` reads contents off an instance that never created any fields; `BaseDialog.defaultHandleControl(...)` doesn't exist (`defaultShellHandling`); and the action sample uses `wActionName` where `ActionBaseDialog` has `wName`, referencing a `GUI_PLUGIN_ELEMENT_PARENT_ID` the class never declares. The working version is already in `annotation-derived-widgets.adoc` under "Transform and action dialogs" — copying it verbatim or linking to it resolves all four. (Same page shows the metadata-editor variant passing `wName` as `lastControl`, where `plugins/metad ata.adoc` passes `null`.) **"Concrete Codebase Examples": 19 of 53 in-repo paths don't resolve.** These pointers are the main thing the guide adds over the existing pages, so they deserve a scripted existence check. Real locations: - `plugins/databases/postgres/` → `postgresql/...databases/postgresql/PostgreSqlDatabaseMeta.java` - `plugins/actions/mail` → `plugins/misc/mail/...mail/workflow/actions/mail/ActionMail.java` (consolidated in #4791) - `plugins/valuetypes/json/.../ValueMetaJson.java` → `core/src/main/java/org/apache/hop/core/row/value/ValueMetaJson.java` - `plugins/metadata/sftp/...` → `plugins/tech/sftp/.../vfs/sftp/metadata/SftpConnection.java` - `PipelineVariableResolver` → `plugins/resolvers/pipeline/.../resolvers/pipeline/VariableResolverPipeline.java` - Azure/Google resolvers sit in `.../core/variables/resolver/` with no trailing `azure`/`google` package (Google's file is `GooleSecretManagerVariableResolver.java`, sic) - `RdbmsExecutionInfoLocation` → `.../execution/database/CachingDatabaseExecutionInfoLocation.java`; `CachingExecutionInfoLocation` → `engine/.../execution/caching/CachingFileExecutionInfoLocation.java` - `TextFileType`/`LogFileType` → `.../explorer/file/types/text/BaseTextExplorerFileType.java` and `.../types/log/LogExplorerFileType.java` - `ConfigPerspective` → `ConfigurationPerspective`; `HopCommandSearch` → `engine/.../search/HopSearch.java`; `ProjectsConfigPlugin` → e.g. `ManageProjectsOptionPlugin` - `HopGuiStartExtensionPoint`, `PipelinePrepareExecutionExtensionPoint`, `HopGuiSearchLocationToolbarItem`, `ExecutionPerspectiveToolbarItem` don't exist — `lineage/xp/LineageHubPipelineCompletedXp.java` and `notifications/NotificationToolbarItem.java` work as substitutes - `plugins/vfs` isn't a category; VFS plugins live under `plugins/tech/*` **Minor:** the `IHopPerspective` sample omits `getControl()` and the `IExecutionInfoLocation` one omits about fifteen non-default methods; a "abbreviated — see the full interface" line would set expectations. -- 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]
