search

joalla / discogs_client

Continuation of the "Official Python Client for the Discogs API"
https://python3-discogs-client.readthedocs.io
Other
310 stars 50 forks source link

Move timeouts and rate-limiting to separate docs chapter #122

Closed JOJ0 closed 1 year ago

JOJ0 commented 1 year ago

What do you think?

Any better ideas for a chapter name? Advanced Configuration? Settings? Advanced Settings?

What I don't like is that it looks pretty similar to "Contribution" since that is right next to it. Maybe just another position in the menu would fix that?

Suggestions welcome. Cheers!

JOJ0 commented 1 year ago

image

AnssiAhola commented 1 year ago

Would it make any difference if we renamed Contribution to Contributing?

image

Other than that I personally like the Configuration Also you mentioned moving it to other position, I think it would make more sense to have Configuration towards the top, maybe after Quickstart or Authentication ?

alifhughes commented 1 year ago

My 2 cents..

I agree with "Contributing". As for the configuration title, I was thinking as both as these are optional configurations, we could call it "Optional Configuration"?

I would keep it after "Authentication" though as auth is one of the most common questions/issues/requests we've had.

Other than thatm I agree with @prcutler's rewording too.

Hope that helps!

JOJ0 commented 1 year ago

https://python3-discogs-client.readthedocs.io/en/docs_config_chapter/

have a look. Enabled the branch (hidden just with link).

What do you think? Kept the position but renamed to optional. Reworded the "timeout sentence".

AnssiAhola commented 1 year ago

Note sure what's up with the tests, maybe change ubuntu-latest to some specific version in https://github.com/joalla/discogs_client/blob/eba0bff930dd36f2583d4de67b002ad69bbfcf20/.github/workflows/discogs_client-build.yml

https://github.com/actions/setup-python/issues/555

JOJ0 commented 1 year ago

Thanks for the hint Anssi! Fixed!

File name was intentionally just configuration.md instead of longer, but you're right, probably it could be confusion when quickly trying to find it by beginning later. Will fix that and merge!