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

feat: Localisation support. #686

Merged
merged 26 commits into from Apr 2, 2022

Conversation

FayeDel
Copy link
Collaborator

@FayeDel FayeDel commented Mar 29, 2022

About

This mega PR:

  • Implements localisation completely, with type-checking, Locale object usage support and documentation support.
  • Implement per-route rate-limit exhaustion on the HTTP client. This also utilises the application/json header correctly.
  • Fixes listener/function removal for Extensions on extension reload/remove.
  • Fixes the timestamp instantiation on the Embed object.
  • Points the OpenCollective URL from the repo to the correct URL.
  • Fixes circular importing from c54a35c
  • Implements the pagination response introduced from GET /guilds/{guild.id}/bans.

Checklist

i0bs and others added 16 commits February 24, 2022 17:03
* docs: add migration docs for 4.1

* docs: detail message content intent in migration
…ctions-py#574)

* docs: add migration docs for 4.1

* docs: detail message content intent in migration
… type conversion safeguard for Locale usage.
@FayeDel FayeDel added bug Something isn't working enhancement New feature or request labels Mar 29, 2022
@FayeDel FayeDel added documentation Improvements or additions to documentation priority This Issue/PR must be resolved first before accepting any others labels Mar 29, 2022
interactions/api/models/message.py Outdated Show resolved Hide resolved
interactions/client.py Outdated Show resolved Hide resolved
interactions/client.pyi Outdated Show resolved Hide resolved
interactions/client.pyi Outdated Show resolved Hide resolved
interactions/client.pyi Outdated Show resolved Hide resolved
interactions/decor.pyi Outdated Show resolved Hide resolved
interactions/decor.pyi Outdated Show resolved Hide resolved
@AstreaTSS
Copy link
Member

...I like this PR in general, but it's kind of odd. Shouldn't these be broken up into separate commits? That would be much clearer when scrolling back through the commit history.

@EdVraz
Copy link
Contributor

EdVraz commented Mar 30, 2022

...I like this PR in general, but it's kind of odd. Shouldn't these be broken up into separate commits? That would be much clearer when scrolling back through the commit history.

I agree on that, I was thinking the same before

@AstreaTSS
Copy link
Member

...I like this PR in general, but it's kind of odd. Shouldn't these be broken up into separate commits? That would be much clearer when scrolling back through the commit history.

I agree on that, I was thinking the same before

Yeah, it's weird for "localization support" to have bug fixes out of all things.
It also makes it harder to review and test.
(Also, yeah, I did mean separate PRs haha 😅.)

@FayeDel
Copy link
Collaborator Author

FayeDel commented Apr 2, 2022

Yeah, it's weird for "localization support" to have bug fixes out of all things.
It also makes it harder to review and test.
(Also, yeah, I did mean separate PRs haha 😅.)

Yeah this PR wasn't meant to be squashed, rather merged/rebased. (But also, I wasn't sure what else to title it 🤷🏻‍♀️)

@AstreaTSS
Copy link
Member

AstreaTSS commented Apr 2, 2022

Yeah, it's weird for "localization support" to have bug fixes out of all things.
It also makes it harder to review and test.
(Also, yeah, I did mean separate PRs haha 😅.)

Yeah this PR wasn't meant to be squashed, rather merged/rebased. (But also, I wasn't sure what else to title it 🤷🏻‍♀️)

That's fine and all and the commit history itself is very clean... but this should really be separated regardless of the merge strategy. It's near impossible to test everything and ensure everything is working fine, and also makes digging through PRs to find what is introduced harder.

IMO there's absolutely no reason this shouldn't be closed and separated into various PRs.

Copy link
Contributor

@EdVraz EdVraz left a comment

Choose a reason for hiding this comment

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

Seems to look good, I can't see anything bad in the latest commits

@FayeDel FayeDel merged commit 4fb0254 into interactions-py:unstable Apr 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request priority This Issue/PR must be resolved first before accepting any others
Projects
Status: Complete
4 participants