Skip to content

feat(GEN-02): migrate GreetCommand to BaseCommandExecutor - #1

Closed
wisdommen wants to merge 1 commit into
masterfrom
feature/gen-02-base-command-executor
Closed

wisdommen wants to merge 1 commit into
masterfrom
feature/gen-02-base-command-executor

Conversation

@wisdommen

@wisdommen wisdommen commented Sep 1, 2026 •

Copy link
Copy Markdown
Member

Summary

Migrates this example plugin's one command class off the deprecated
AbstractCommandExecutor onto the current BaseCommandExecutor, ahead
of UltiTools-API 6.3.0 removing the deprecated class entirely
(GEN-02).

This repository exists as the canonical worked example of the
External Plugin API (UltiToolsAPI.connect(this) from a plain Bukkit
JavaPlugin) — leaving it on the removed base class would ship a
worked example that no longer compiles against 6.3.0.

What changed

  • GreetCommand.java — two-line diff only: the import
    com.ultikits.ultitools.abstracts.AbstractCommandExecutor becomes
    com.ultikits.ultitools.abstracts.command.BaseCommandExecutor, and
    extends AbstractCommandExecutor becomes extends BaseCommandExecutor.

Behaviour preservation

  • Sender restrictions unchanged. GreetCommand carries an explicit
    class-level @CmdTarget(CmdTarget.CmdTargetType.BOTH), preserved
    verbatim. Both AbstractCommandExecutor and BaseCommandExecutor
    resolve @CmdTarget through the identical
    CmdTargetComposition.resolve(...) call, so the base-class swap alone
    cannot change the resolved sender type.
  • Every @CmdExecutor alias/permission/description and every
    @CmdMapping format string is byte-identical
    to before — confirmed
    by git diff, which touches only the import line and the extends
    clause.
  • handleHelp(CommandSender) is unchanged. The class already
    implemented this method before the migration; BaseCommandExecutor
    makes it a protected abstract contract, and the existing
    (non-empty) body satisfies it with no stub.
  • External Plugin API registration path verified, not assumed.
    This plugin registers via UltiToolsAPI.connect(this) →
    CommandManager.registerAllExternal(ExternalPluginAdapter), which
    resolves command beans by CommandExecutor type from the adapter's
    IoC container (no cast to AbstractCommandExecutor anywhere on this
    path) — confirmed by reading CommandManager.java directly. The
    migrated class continues to register on the exact path this example
    exists to demonstrate.

Build

  • Compiles and builds green against the existing, released
    UltiTools-API 6.2.2 pin — no snapshot repository added, no version
    bump to the framework dependency (D-10 round 1). mvn -B -f pom.xml test: BUILD SUCCESS (no test sources exist in this repo).
  • No version bump to this project's own pom.xml — this repository has
    no .github/workflows/ directory at all, so it has no publish
    mechanism and merging it releases nothing.

Scope note

Unifying the UltiTools-API pin to 6.3.0 is a separate, later step
(D-10 round 2), explicitly out of scope here.

Not merging in this PR — awaiting the monorepo Ship Gate's pre-merge
gates (own code review, third-party review, real-machine UAT, green
CI).

Summary by CodeRabbit

  • Refactor
    • Updated the greeting command to use the current command execution framework.
    • No user-facing behavior changes are expected.

⛔ HOLD — do not merge until UltiTools-API 6.3.0 is released

Maintainer decision, 2026-09-02. This is not a review objection; the migration itself is correct.

Why holding. BaseCommandExecutor.suggest() in the released UltiTools-API 6.2.1 that this
module pins only suggests first-token literals — it never resolves @CmdParam(suggest = "...").
Verified by disassembling the published artifact: inside suggest(Player, Command, String[]) there
are zero references to CmdParam, against 7 elsewhere in the same class (a live control group),
while @CmdParam at 6.2.1 does declare a suggest() member. The deprecated AbstractCommandExecutor
this PR migrates away from did resolve it.

So merging and publishing now would ship a module whose declared tab-completion silently stops
working on every server still running a 6.2.x framework. The fix (CommandTabCompletionDispatch)
exists only in the unreleased 6.3.0.

The ordering that avoids the regression entirely: this PR merges and publishes as part of the
6.3.0 release, so the framework and the module land together. Downstream publication is already
recorded as a hard precondition of the 6.3.0 framework release gate in the framework repo's Phase 7
artifacts, so holding here is consistent with that, not a new deferral.

Still outstanding before merge: Ship Gate gate 3 (Laojun real-machine UAT) has not been run for
this PR. Gate 1 (own code review) and gate 4 (CI) are green; gate 2 reported CodeRabbit "Review
completed".

Swap the deprecated AbstractCommandExecutor for the current
BaseCommandExecutor (import + extends only). All @CmdTarget,
@cmdexecutor, @CmdMapping and @CmdParam annotations are preserved
verbatim, and the existing handleHelp(CommandSender) override is
kept unchanged, satisfying BaseCommandExecutor's protected abstract
contract with no stub.

Compiles against the existing released UltiTools-API 6.2.2 pin,
no snapshot repository added (D-10 round 1).
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-01T12:37:13.745635Z e128f2f PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 09d4243f-1583-4453-9fbc-562520618152

📥 Commits

Reviewing files that changed from the base of the PR and between 39940d5 and e128f2f.

📒 Files selected for processing (1)
  • src/main/java/com/example/ultitoolsext/commands/GreetCommand.java

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

GreetCommand now extends BaseCommandExecutor instead of AbstractCommandExecutor. The import was updated to match the new base class package.

Changes

GreetCommand Update

Layer / File(s) Summary
Update GreetCommand executor base
src/main/java/com/example/ultitoolsext/commands/GreetCommand.java
The import and class declaration now use BaseCommandExecutor.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: 🔵 Low · up to e128f

The PR only changes the command’s framework base class while preserving its permissions, sender targets, aliases, mappings, and handlers, and it builds successfully against API 6.2.2. Merge is reasonable with explicit owner awareness that compatibility between the old and new base classes should be confirmed before relying on this example with API 6.3.0.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: migrating GreetCommand to BaseCommandExecutor.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/gen-02-base-command-executor

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@wisdommen

Copy link
Copy Markdown
Member Author

The commits from this PR were carried into #5 with git cherry-pick -x, preserving authorship (e128f2f → 9cb2317); closing in favour of #5.

本 PR 的提交已通过 git cherry-pick -x 保留作者信息并入 #5,因此关闭本 PR,改由 #5 合入。

@wisdommen wisdommen closed this Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant