-
Notifications
You must be signed in to change notification settings - Fork 149
Improve Example documentation. #610
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
Conversation
@Picsou993
Picsou993
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original messages are already accurate and concise information. It's also not needed to be restrictive in the comment when the restriction is not something done in the code.
hypixel-api-example/src/main/java/net/hypixel/api/example/GetGuildExample.java
Outdated
Show resolved
Hide resolved
@Picsou993
Picsou993
Aug 8, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original message is correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They have the same meaning but the edit is more grammatically correct and imo roles off the tongue easier.
@jerryxfu
jerryxfu
Aug 8, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The correct way to say it is, I believe, to drop the comma after "name" and use members' instead of member's
We'll be fetching the guild's stats by its ID for this example, but guilds can
also be looked up using their name or one of their members' Minecraft UUIDs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"members" should definitely be plural, since the alternative would be "one of their member". As for "members'" vs "members's", I think it's just a matter of style, but the former is normally preferred (Firefox's spellcheck highlights the latter, if that counts for anything).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@Picsou993
Picsou993
Aug 8, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This 125 members limitation is only an in-game limitation, the endpoint could technically returns more members if there was more players in the guild.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this better in reflected with cefffd2?
hypixel-api-example/src/main/java/net/hypixel/api/example/GetGuildExample.java
Show resolved
Hide resolved
@Picsou993
Picsou993
Aug 8, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original message should be kept as we are here losing the fact that the performances are dependant of the API response time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current thread being blocked until something happening (in this case the API responding) is the definition of synchronous operation(which dependent the time the API takes to respond is how long the thread takes to be unblocked hence the performance being dependent on api response time)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Hypixel API is very accessible, so it often attracts a lot of new developers, which I catered these examples toward. From a newbie's perspective, I worry that "synchronous" might send them down a rabbit hole that makes their learning curve feel a lot steeper. Words like "blocks" and "thread" are there so that those who understand those concepts get the gist.
125 is a in game limit on how many people can join a guild in hypixel, the endpoint can theoretically return infinite.
hypixel-api-example/src/main/java/net/hypixel/api/example/GetGuildExample.java
Outdated
Show resolved
Hide resolved
I also added a bit to the README..
hypixel-api-example/README.md
Outdated
@jerryxfu
jerryxfu
Aug 8, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Response data include the player's UUID, network level, rank, client version [<- confirm that], and more!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! I wrote GetGuildExample, so I left some clarification in the conversations for this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Hypixel API is very accessible, so it often attracts a lot of new developers, which I catered these examples toward. From a newbie's perspective, I worry that "synchronous" might send them down a rabbit hole that makes their learning curve feel a lot steeper. Words like "blocks" and "thread" are there so that those who understand those concepts get the gist.
hypixel-api-example/src/main/java/net/hypixel/api/example/GetGuildExample.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"members" should definitely be plural, since the alternative would be "one of their member". As for "members'" vs "members's", I think it's just a matter of style, but the former is normally preferred (Firefox's spellcheck highlights the latter, if that counts for anything).
@TheNullicorn as to "members' " vs "members's", I was unaware of the 1st being a convention, and so corrected it and addressed synchronous vs thread blocking in d4f220d, is this language better?
I felt the urge to make the comments more concise, and in some cases more specific 😅