Top-gg-Community / python-sdk

A simple API wrapper for top.gg written in Python
https://docs.top.gg/docs/Libraries/python
MIT License
91 stars 54 forks source link

Issues with the Wrapper and Example #2

Closed fourjr closed 6 years ago

fourjr commented 6 years ago

Example

Wrapper

MattIPv4 commented 6 years ago

Additionally to comply to PEP8 which an example really should, you should have two linebreaks before your class declaration. And following d.py best practices your file at the top and class should have descriptions.

MattIPv4 commented 6 years ago

In the example I would also demonstrate hooking into the guild join/leave events and the on ready event for posting stats.

fourjr commented 6 years ago

It shouldn't hook into join/leave imo, because of API ratelimits.

MattIPv4 commented 6 years ago

As this is a full library for API contact, I would personally expect the library to handle ratelimiting itself so that you can do what you want and it will stop you getting api blocked by doing its own ratelimiting. This would then allow you to safely hook into join/leave.

franc-ist commented 6 years ago

With regards to get_server_count, that function is there to also get the server count of other bots (if need be), so personally I feel getting the id from the passed bot object wouldn't be useful in this case.

tonkku107 commented 6 years ago

Hooking into join/leave is not necessary. Hooking like the js library is now preferred. js lib does every 30 minutes

On Mon, Feb 12, 2018, 00:50 Matt (IPv4), notifications@github.com wrote:

As this is a full library for API contact, I would personally expect the library to handle ratelimiting itself so that you can do what you want and it will stop you getting api blocked by doing its own ratelimiting. This would then allow you to safely hook into join/leave.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/DiscordBotList/DBL-Python-Library/issues/2#issuecomment-364797505, or mute the thread https://github.com/notifications/unsubscribe-auth/AENItLwd-7RIEbLrhRoGsJmEOIQduYGCks5tT262gaJpZM4SBi47 .

fourjr commented 6 years ago

@FishyFing, Bot ID can be an optional argument but it defaults to the current bot

fourjr commented 6 years ago

Refer to #3

franc-ist commented 6 years ago

TODO: