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

Rename category -> itemGroup #3232

Merged
merged 4 commits into from
Sep 6, 2021

Conversation

svr333
Copy link
Member

@svr333 svr333 commented Sep 4, 2021

Description

This is a follow up to #3139, a lot of comments and variables and error messages needed to be updated still.
This is what that does

Two more follow up questions:

  • in wiki.json, Christmas-Seasonal-Category is mentioned, change to Christmas-Seasonal-ItemGroup?
  • do I update messages.yml (and other files) keys? (eg: guide.tooltips.open-category -> guide.tooltips.open-itemgroup)

Checklist

  • I have fully tested the proposed changes and promise that they will not break everything into chaos.
  • I have also tested the proposed changes in combination with various popular addons and can confirm my changes do not break them.
  • I followed the existing code standards and didn't mess up the formatting.
  • I did my best to add documentation to any public classes or methods I added.
  • I have added Nonnull and Nullable annotations to my methods to indicate their behaviour for null values
  • I added sufficient Unit Tests to cover my code.

@svr333 svr333 requested a review from a team as a code owner September 4, 2021 10:56
@TheBusyBiscuit TheBusyBiscuit added the 🧹 Chores Refactoring / Cleanup. label Sep 5, 2021
Copy link
Member

@TheBusyBiscuit TheBusyBiscuit left a comment

Choose a reason for hiding this comment

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

Looks good, just one small mistake.

Thanks!

@TheBusyBiscuit TheBusyBiscuit self-assigned this Sep 5, 2021
@svr333
Copy link
Member Author

svr333 commented Sep 5, 2021

Also what about these @TheBusyBiscuit

  • in wiki.json, Christmas-Seasonal-Category is mentioned, change to Christmas-Seasonal-ItemGroup?
  • do I update messages.yml (and other files) keys? (eg: guide.tooltips.open-category -> guide.tooltips.open-itemgroup)

@TheBusyBiscuit
Copy link
Member

Also what about these @TheBusyBiscuit

  • in wiki.json, Christmas-Seasonal-Category is mentioned, change to Christmas-Seasonal-ItemGroup?
  • do I update messages.yml (and other files) keys? (eg: guide.tooltips.open-category -> guide.tooltips.open-itemgroup)

Regarding the wiki reference, the wiki page itself would need to be renamed first (if thats wanted).
Message strings... well, they could be updated but they would need to be updated in every class and every locale file. If you wanna go through all of that, sure. Otherwise we can just leave it as is or do it later.

@svr333
Copy link
Member Author

svr333 commented Sep 6, 2021

I think I'm going to keep the Christmas seasonal category as-is, because I think the in-game text also still says category over itemgroup, and in this case it does make more sense
its a christmas category

Ill do the other changes in a bit

Copy link
Member

@WalshyDev WalshyDev left a comment

Choose a reason for hiding this comment

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

God GitHub hates large PRs but I couldn't find any typos or anything so looks good

@TheBusyBiscuit TheBusyBiscuit merged commit fc207b6 into Slimefun:master Sep 6, 2021
@svr333 svr333 deleted the FixCategoryNaming branch September 6, 2021 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧹 Chores Refactoring / Cleanup.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants