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.

Sign up for GitHub

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

Jump to bottom

The End of the Getter era #7285

Merged
merged 20 commits into from
Dec 31, 2024
Merged

The End of the Getter era #7285

merged 20 commits into from
Dec 31, 2024

Conversation

Efnilite
Copy link
Member

@Efnilite Efnilite commented Dec 19, 2024 •
edited
Loading

Description

Removes Getter from BukkitEventValues and all other classes.

Target Minecraft Versions: any
Requirements: none
Related Issues: none

@Efnilite Efnilite added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Dec 19, 2024
sovdeeth
sovdeeth requested changes Dec 19, 2024
View reviewed changes
Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only looked at a few but it applies to everything

src/main/java/ch/njol/skript/expressions/ExprFoodLevel.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/expressions/ExprTool.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/expressions/ExprWeather.java Outdated Show resolved Hide resolved
@Efnilite Efnilite requested a review from sovdeeth December 20, 2024 15:35
APickledWalrus
APickledWalrus requested changes Dec 20, 2024
View reviewed changes
Copy link
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good. In BukkitEventValues, there are cases where pattern matching, and TIME constants can be used. I highlighted one of each I noticed.

src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java Outdated Show resolved Hide resolved
TheAbsolutionism
TheAbsolutionism suggested changes Dec 20, 2024
View reviewed changes
Copy link
Contributor

@TheAbsolutionism TheAbsolutionism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be cool if you updated these while you're at it

src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java Show resolved Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java Outdated Show resolved Hide resolved
@Efnilite Efnilite added housekeeping Housekeeping-related issue or task. and removed enhancement Feature request, an issue about something that could be improved, or a PR improving something. labels Dec 21, 2024
Burbulinis
Burbulinis approved these changes Dec 21, 2024
View reviewed changes
Copy link

@Burbulinis Burbulinis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for your service, getters!

@sovdeeth sovdeeth added the 2.10 Targeting a 2.10.X version release label Dec 29, 2024
@Moderocky
Copy link
Member

You almost reintroduced the ignite block issue 😬

@Moderocky Moderocky merged commit fb24b27 into SkriptLang:dev/feature Dec 31, 2024
5 checks passed
@Efnilite Efnilite deleted the end-of-the-getter-era branch December 31, 2024 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Burbulinis Burbulinis Burbulinis approved these changes

@TheAbsolutionism TheAbsolutionism TheAbsolutionism approved these changes

@APickledWalrus APickledWalrus APickledWalrus approved these changes

@Moderocky Moderocky Moderocky approved these changes

@sovdeeth sovdeeth sovdeeth approved these changes

Assignees
No one assigned
Labels
2.10 Targeting a 2.10.X version release housekeeping Housekeeping-related issue or task.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Efnilite @Moderocky @sovdeeth @APickledWalrus @TheAbsolutionism @Burbulinis