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

refactor!: use attrs.asdict as a way to get json data of the object #1126

Merged
merged 39 commits into from Nov 23, 2022

Conversation

Damego
Copy link
Member

@Damego Damego commented Oct 17, 2022

About

This pull request:

  1. Makes DictSerializerMixin._json as a property with using attrs.asdict to get json data of the object.
  2. Solves all troubles with editing json. You don't need to edit json anymore lol.
  3. Removes ComponentMixin and json modifing in a lot of objects like Embed.
  4. Limited using _json property in some places: using walrus operators and objects instead of dicts(wsclient).
  5. Removes outdated(since attrs added) code like here

Checklist

  • The pre-commit code linter has been run over all edited files to ensure the code is linted.
  • I've ensured the change(s) work on 3.8.6 and higher.

I've made this pull request: (check all that apply)

  • For the documentation
  • To add a new feature
  • As a general enhancement
  • As a refactor of the library/the library's code
  • To fix an existing bug
  • To resolve #ISSUENUMBER

This is:

  • A breaking change

@Damego Damego marked this pull request as ready for review October 19, 2022 08:20
@Damego
Copy link
Member Author

Damego commented Oct 19, 2022

Ready for review. Any tests appreciated

@EdVraz
Copy link
Contributor

EdVraz commented Oct 19, 2022

Do not merge for another few days as this is a major change

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.

Optimized code in some places: using walrus operators and objects instead of dicts(wsclient).

Out of the scope of this PR, please remove

@Damego
Copy link
Member Author

Damego commented Oct 19, 2022

Optimized code in some places: using walrus operators and objects instead of dicts(wsclient).

Out of the scope of this PR, please remove

Remove changes or just these words?

@EdVraz
Copy link
Contributor

EdVraz commented Oct 19, 2022

Optimized code in some places: using walrus operators and objects instead of dicts(wsclient).

Out of the scope of this PR, please remove

Remove changes or just these words?

The changes

@Damego Damego removed the on hold This issue/PR is on hold pending higher priority issues/PRs label Nov 6, 2022
@Damego Damego marked this pull request as ready for review November 6, 2022 17:16
interactions/api/gateway/client.py Show resolved Hide resolved
interactions/client/models/component.py Outdated Show resolved Hide resolved
interactions/client/models/component.py Outdated Show resolved Hide resolved
interactions/client/models/component.py Outdated Show resolved Hide resolved
interactions/client/models/component.py Outdated Show resolved Hide resolved
interactions/client/models/component.py Outdated Show resolved Hide resolved
interactions/utils/attrs_utils.py Outdated Show resolved Hide resolved
Damego and others added 10 commits November 7, 2022 08:13
Co-authored-by: EdVraz <88881326+EdVraz@users.noreply.github.com>
Co-authored-by: EdVraz <88881326+EdVraz@users.noreply.github.com>
Co-authored-by: EdVraz <88881326+EdVraz@users.noreply.github.com>
Co-authored-by: EdVraz <88881326+EdVraz@users.noreply.github.com>
Co-authored-by: EdVraz <88881326+EdVraz@users.noreply.github.com>
@EdVraz
Copy link
Contributor

EdVraz commented Nov 10, 2022

Also merge conflicts

@Damego
Copy link
Member Author

Damego commented Nov 10, 2022

Resolved

@Damego Damego requested a review from EdVraz November 10, 2022 12:38
@EdVraz EdVraz changed the title refactor: use attrs.asdict as a way to get json data of the object refactor!: use attrs.asdict as a way to get json data of the object Nov 17, 2022
@mAxYoLo01
Copy link
Contributor

Tested PR and works on my end, LGTM!

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.

Since the other devs don't seem interested in having a look at this PR I'm approving it

@EdVraz EdVraz merged commit a23d25e into interactions-py:unstable Nov 23, 2022
@Damego Damego deleted the why-didn't-we-do-this-before branch December 6, 2022 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants