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

Rewrite dblstatistics.js to new TopStats API functionality #1

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

Conversation

Luke-6723
Copy link
Member

No description provided.

@Xignotic84 Xignotic84 self-requested a review October 28, 2024 11:38
lib/index.ts Outdated Show resolved Hide resolved
Copy link
Member Author

@Luke-6723 Luke-6723 left a comment

Choose a reason for hiding this comment

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

Looking good!

README.md Outdated Show resolved Hide resolved
lib/index.ts Show resolved Hide resolved
lib/index.ts Outdated Show resolved Hide resolved
lib/index.ts Outdated Show resolved Hide resolved
lib/index.ts Outdated
Comment on lines 43 to 51
approved_at: string;
/** Current monthly votes */
monthly_votes: number;
/** Current server count */
server_count: number;
/** Total votes received */
total_votes: number;
/** Current shard count */
shard_count: number;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The mix of camel case and snake case would make the experience inconsistent.
It's my understanding that the API returns snake case but the internal pieces of the library use camel case.
It may be not worth the effort but getting them to be the same throughout the library would make this much nicer. Auto conversion?

lib/index.ts Outdated Show resolved Hide resolved
lib/index.ts Show resolved Hide resolved
lib/index.ts Outdated Show resolved Hide resolved
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