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 json endpoint for hiscores #2

Closed
wants to merge 2 commits into from

Conversation

pjoeterbliep
Copy link

Tested it, seems to work.

@DrYoshiyahu
Copy link
Contributor

Amazing! What a quick turnaround! <3

@leejt
Copy link

leejt commented Aug 30, 2024

Is there a reason we want to support both endpoints? Can you just replace the existing functionality rather than supporting both json and ws?

@wgevaert
Copy link

Backwards compatibility mostly. You dont want all tools to break when we deploy this right?

Or maybe you do

Code would for sure be simpler if we allow only one endpoint

@pjoeterbliep
Copy link
Author

I made a pull request that breaks backwards compatibility here, it also fixes issue 1. We can merge one of the two and close the other.

@DrYoshiyahu
Copy link
Contributor

I think it's completely fine to break backwards compatibility and forget the ws endpoint. There are so few tools that actually use this plugin, and it's a very easy fix.

@pjoeterbliep
Copy link
Author

pjoeterbliep commented Sep 5, 2024

Closed in favor of #3
Consensus seems to be to break backwards compatibility while implementing this feature, which is what that pull request does

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants