Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid deprecated CompoundTag in API #2883

Merged
merged 2 commits into from
Sep 14, 2024
Merged

Conversation

SirYwell
Copy link
Member

Overview

Description

CompoundTag is deprecated and its implementation is now backed by LinCompoundTag. This causes significant overhead when access its data due to the needed transformation/copy.

This PR aims to address spots where CompoundTag is used in FAWE api, as well as some uses that can benefit from the reduced overhead. There is more to do to get CompoundTag fully out of FAWE, but more changes can be addressed later.

Submitter Checklist

@SirYwell SirYwell requested a review from a team as a code owner August 18, 2024 11:44
@github-actions github-actions bot added the chore label Aug 18, 2024
@NotMyFault NotMyFault requested a review from a team August 22, 2024 16:30
@dordsor21
Copy link
Member

Was moving to jetbrains annotations from javax done on purpose? I believe we're relatively consistent for the latter

@NotMyFault
Copy link
Member

Was moving to jetbrains annotations from javax done on purpose? I believe we're relatively consistent for the latter

JavaX is superseded by Jakarta. A migration away from JavaX is a good choice, whether it's its successor or a replacement.

@dordsor21
Copy link
Member

Was moving to jetbrains annotations from javax done on purpose? I believe we're relatively consistent for the latter

JavaX is superseded by Jakarta. A migration away from JavaX is a good choice, whether it's its successor or a replacement.

Should probably make a decision and move all of them at once imo

@SirYwell
Copy link
Member Author

SirYwell commented Sep 1, 2024

Was moving to jetbrains annotations from javax done on purpose? I believe we're relatively consistent for the latter

No, it's mostly because IntelliJ likes to use its annotations by default. I switched to javax for now, but we might want to consider switching to jspecify or checkerframework in future.

Copy link

Please take a moment and address the merge conflicts of your pull request. Thanks!

@NotMyFault NotMyFault merged commit 1e8778b into main Sep 14, 2024
11 checks passed
@NotMyFault NotMyFault deleted the refactor/avoid-CompoundTag branch September 14, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants