🤖 refactor!: remove the deprecated v1 API surface#104
Merged
Conversation
Goal: remove the entire @deprecated(forRemoval = true) API surface from LightKeeper. Plan: (1) drop the tests that only pin the deprecated surface, (2) remove the v1 facade delegates, (3) remove the Vector3Di vocabulary, (4) remove CapturedEventSnapshot.data(), (5) refresh the README. This is step 1. Dropping these pins first keeps every subsequent removal commit green: nothing left references the deprecated members once the surface is gone. - Delete DeprecatedDelegateTest entirely: it exists solely to pin that each v1 default method delegates to its facet replacement. The facet behaviour itself is covered by the facade/facet tests, so no real coverage is lost. - Drop the deprecated-overload pins whose canonical BlockPos paths are already covered: WorldHandleTest (blockTypeAt/setBlockAt Vector3Di), PlayerHandleTest (leftClickBlock Vector3Di), HandleAssertionsTest (hasBlockAt Vector3Di). - Drop CapturedEventSnapshotTest.data(): pins the deprecated raw-string view; values()/value() coverage stays. - Remove the now-orphaned @SuppressWarnings("removal") and unused imports. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn
Goal: remove the entire @deprecated(forRemoval = true) API surface. Plan step 2: strip the flat v1 delegate methods from ILightkeeperFramework now that their pins are gone. Removes the 20 deprecated default methods that merely forwarded into the facet accessors: mainWorld, newWorld x2, newWorldFromTemplate, createPlayer x2, buildPlayer, buildWorld, executeCommand, serverOutput, platform, serverDirectory, pluginDataDirectory, crashServer, stopServer, startServer, restartServer, captureEvents, currentServerTick, and serverErrors. The facet accessors server()/worlds()/bots()/events() are the API now and stay, alongside waitUntil() and close(). DefaultLightkeeperFramework.currentServerTick() is retained untouched: it is the gateway seam mandated by IFrameworkGatewayView (consumed by ServerControlFacade), not the deprecated interface default. Drops the type-javadoc deprecation note and the imports orphaned by the removal. BREAKING CHANGE: the flat v1 methods on ILightkeeperFramework are removed; use the facet accessors (server()/worlds()/bots()/events()) instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn
Goal: remove the entire @deprecated(forRemoval = true) API surface. Plan step 3: drop the legacy Vector3Di block-coordinate type and every overload that only existed to accept it, now that BlockPos is the sole vocabulary. - Delete the Vector3Di record and its toBlockPos() bridge. - PlayerHandle: remove the four Vector3Di leftClickBlock/rightClickBlock overloads (single-arg and with BlockFace). - WorldHandle: remove the blockTypeAt(Vector3Di) and setBlockAt(Vector3Di, String) overloads. - WorldHandleAssert: remove hasBlockAt(Vector3Di) and its now-unused import. The canonical BlockPos overloads are unchanged and already carried the coverage. BREAKING CHANGE: Vector3Di and every overload accepting it are removed; use the BlockPos overloads instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn
Goal: remove the entire @deprecated(forRemoval = true) API surface. Plan step 4: drop the deprecated raw-string data() view now that its pin is gone. - Remove data(): the string-rendered view of the captured values. The typed values() map and value(String) lookup are the API and stay. - Repoint the tick javadoc from the removed ILightkeeperFramework#currentServerTick() to the canonical IServerControl#currentTick(). BREAKING CHANGE: CapturedEventSnapshot.data() is removed; use values() (typed) or value(String) instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn
Goal: remove the entire @deprecated(forRemoval = true) API surface. Plan step 5: the README no longer needs to steer readers away from the removed flat method names, so replace the deprecation note with a plain facet-orientation pointer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn
Owner
Author
|
@coderabbitai full review |
Owner
Author
|
@codex review |
Contributor
✅ Action performedFull review finished. |
Contributor
📝 WalkthroughWalkthroughThe PR removes deprecated flat framework methods, the rendered event snapshot map accessor, and Vector3Di-based world, player, and assertion APIs. Documentation and tests are updated to reflect facet accessors, typed event values, and BlockPos-based operations. ChangesFramework API cleanup
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the deprecated v1 “flat” API surface from the LightKeeper JUnit framework module, leaving the facet-based API (server()/worlds()/bots()/events()) as the only public entrypoint for external consumers.
Changes:
- Deleted deprecated default delegates from
ILightkeeperFramework, enforcing facet-only access. - Removed the deprecated
Vector3Dicoordinate vocabulary and its overloads across handles/assertions/tests (canonicalBlockPosonly). - Removed deprecated
CapturedEventSnapshot.data()string-rendered view and the tests that existed solely to pin deprecated behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates docs to point exclusively at the facet accessors. |
| lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/ILightkeeperFramework.java | Removes deprecated v1 delegate methods, leaving only facet accessors + waitUntil/close. |
| lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/Vector3Di.java | Deletes the deprecated coordinate type in favor of BlockPos. |
| lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/WorldHandle.java | Removes deprecated Vector3Di overloads for block operations. |
| lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/PlayerHandle.java | Removes deprecated Vector3Di overloads for block interaction operations. |
| lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/CapturedEventSnapshot.java | Removes deprecated data() view; keeps typed values() and value(String). |
| lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/assertions/WorldHandleAssert.java | Removes deprecated Vector3Di assertion entrypoint. |
| lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/DeprecatedDelegateTest.java | Deletes tests that pinned deprecated v1 delegation behavior. |
| lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/WorldHandleTest.java | Removes deprecated Vector3Di overload delegation tests. |
| lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/PlayerHandleTest.java | Removes deprecated Vector3Di overload delegation test. |
| lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/CapturedEventSnapshotTest.java | Removes test for deprecated data() view. |
| lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/assertions/HandleAssertionsTest.java | Removes deprecated Vector3Di assertion delegation test/import. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/CapturedEventSnapshot.java (1)
22-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
{@link}instead of{@code}for Javadoc cross-references.Using
{@linkIServerControl#currentTick()}creates a navigable link in the generated documentation, whereas{@code}only formats the text as code without a hyperlink.♻️ Proposed refactor
- * {`@code` IServerControl#currentTick()}. + * {`@link` IServerControl#currentTick()}.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/CapturedEventSnapshot.java` at line 22, Update the Javadoc reference to IServerControl#currentTick() in CapturedEventSnapshot by using a link tag instead of a code tag, preserving the referenced method and surrounding documentation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/CapturedEventSnapshot.java`:
- Line 22: Update the Javadoc reference to IServerControl#currentTick() in
CapturedEventSnapshot by using a link tag instead of a code tag, preserving the
referenced method and surrounding documentation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 751134e4-724f-40aa-a43a-e1890f78e8d3
📒 Files selected for processing (12)
README.mdlightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/CapturedEventSnapshot.javalightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/ILightkeeperFramework.javalightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/PlayerHandle.javalightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/Vector3Di.javalightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/WorldHandle.javalightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/assertions/WorldHandleAssert.javalightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/CapturedEventSnapshotTest.javalightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/DeprecatedDelegateTest.javalightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/PlayerHandleTest.javalightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/WorldHandleTest.javalightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/assertions/HandleAssertionsTest.java
💤 Files with no reviewable changes (9)
- lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/assertions/HandleAssertionsTest.java
- lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/Vector3Di.java
- lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/WorldHandleTest.java
- lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/PlayerHandleTest.java
- lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/WorldHandle.java
- lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/assertions/WorldHandleAssert.java
- lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/CapturedEventSnapshotTest.java
- lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/PlayerHandle.java
- lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/DeprecatedDelegateTest.java
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
How
@Deprecated(forRemoval = true)v1 default delegates fromILightkeeperFramework, the entireVector3Divocabulary (record + every overload onPlayerHandle/WorldHandle/WorldHandleAssert), andCapturedEventSnapshot.data().DefaultLightkeeperFramework.currentServerTick()stays — it is the gateway-seam implementation (IFrameworkGatewayView), not a deprecated delegate.Tests
mvn --batch-mode -Perrorprone clean verify checkstyle:checkstyle pmd:check→ BUILD SUCCESS (full reactor, all 20 IT classes green on both lanes).rg "forRemoval"→ zero matches.BREAKING CHANGE: the v1 flat method names, Vector3Di, and CapturedEventSnapshot.data() are gone; use the facet API, BlockPos, and values()/value().
🤖 Generated with Claude Code
Summary by CodeRabbit
Breaking Changes
Vector3Ditype and related overloads. UseBlockPosfor block interactions and assertions.data()view; typed captured values remain available.Documentation