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

Update grakkit-minestom to support recent Minestom version #38

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Update grakkit-minestom to support recent Minestom version #38

wants to merge 7 commits into from

Conversation

para-dise
Copy link

This request aims at fixing a few things

  • Re-locate extension.json from META-INF to root folder
  • Remove LoadStatus as it was removed
  • Remove use of deprecated TimeUnit.TICK
  • Adds a minestom option for gradle
  • Rename the "paper" build name when you're actually building Minestom
  • Updates Minestom commit

@para-dise
Rename paper to minestom
d79d280 
Fixes the incorrect "paper" name when building minestom
@para-dise
Delete incorrect extension.json location
d9d2cb7 
This file should've been moved out of META-INF
@para-dise
Create extension.json
c8a2629 
Add the required extension.json for Grakkit in the right folder
@para-dise
Fix loading and deprecation warnings
4feafbf 
Remove LoadStatus and replaces TimeUtils.TICK with TaskSchedule
@para-dise
Add minestom to settings.gradle
b2e2b86
@para-dise
Update to recent Minestom commit
afaf895
ghost
ghost reviewed Sep 12, 2022
View reviewed changes
Copy link

@ghost ghost left a comment •
edited by ghost

Choose a reason for hiding this comment

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

This overall looks solid, though I did notice that, in Main.java, you're creating a second scheduler manager in the initialize method when one already exists as the private schedulerManager value on the class. @MercerK your thoughts?

@para-dise
Remove unnecessary scheduler
0bf5ad1 
Removed an extra getSchedulerManager call
Removed unused imports
@para-dise
Copy link
Author

This overall looks solid, though I did notice that, in Main.java, you're creating a second scheduler manager in the initialize method when one already exists as the private schedulerManager value on the class. @MercerK your thoughts?

Whoops, didn't see that, fixed in the latest commit

@MercerK
Copy link
Contributor

MercerK commented Sep 12, 2022

@Mythical-Forest-Collective Might have more feedback. Looks fine otherwise.

@MercerK
Copy link
Contributor

MercerK commented Oct 13, 2022

Did we want to merge this in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@para-dise @MercerK