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

Add fetching MC versions and Console from the extensions API #4168

Merged
merged 9 commits into from
Jan 24, 2024

Conversation

rtm516
Copy link
Member

@rtm516 rtm516 commented Oct 1, 2023

No description provided.

@rtm516 rtm516 added the PR: Feature When a PR implements a new feature label Oct 1, 2023
Konicai
Konicai previously requested changes Oct 1, 2023
Copy link
Member

@Konicai Konicai left a comment

Choose a reason for hiding this comment

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

all of the other methods in GeyserApi are fluent

@onebeastchris onebeastchris added the API The issue/feature request relates to the Geyser API label Oct 7, 2023
@Konicai
Copy link
Member

Konicai commented Dec 6, 2023

Needs rebase or merge

@onebeastchris onebeastchris added the PR: Needs review Indicates that a PR is functional and review-ready. label Dec 19, 2023
Copy link
Member

Choose a reason for hiding this comment

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

Might be a better idea to create a new impl package or something and move this file there.

Copy link
Member

Choose a reason for hiding this comment

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

I think I would've placed this file under org.geysermc.geyser.api.util as it's also under the util package of the api module.

Copy link
Member

Choose a reason for hiding this comment

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

4df0458 moved it to a new impl package. Not sure how org.geysermc.geyser.api.util would work as that would end up in the api package alongside the interface it implements?

Comment on lines +139 to +143
/**
* Gets the version of Java Minecraft that is supported.
*
* @return the supported version of Java Minecraft
*/
Copy link
Member

Choose a reason for hiding this comment

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

If we were to follow the Google Style Guide it'd be fine to simplify this to:
Returns the version of Java Minecraft that is supported

Copy link
Member

Choose a reason for hiding this comment

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

Not against doing that, i've mostly mirrored the style of existing method docs

@onebeastchris onebeastchris dismissed Konicai’s stale review January 24, 2024 19:03

Addressed, methods are fluent now

@onebeastchris onebeastchris merged commit 3f577f4 into GeyserMC:master Jan 24, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API The issue/feature request relates to the Geyser API PR: Feature When a PR implements a new feature PR: Needs review Indicates that a PR is functional and review-ready.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants