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: implement Member.id #465

Merged
merged 5 commits into from Feb 5, 2022
Merged

feat: implement Member.id #465

merged 5 commits into from Feb 5, 2022

Conversation

Toricane
Copy link
Contributor

@Toricane Toricane commented Feb 4, 2022

About

This pull request is about implementing interactions.Member.id, which lets you use member.id instead of member.user.id.
Another example is ctx.author.id instead of ctx.author.user.id.

Checklist

  • I've ran pre-commit to format and lint the change(s) made.
  • I've checked to make sure the change(s) work on 3.8.6 and higher.
  • This fixes/solves an Issue.
    • (If existent):
  • I've made this pull request for/as: (check all that apply)
    • Documentation
    • Breaking change
    • New feature/enhancement
    • Bugfix

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.

This does not reflect the current api scheme, which is not including the ID into the member object

Because of this, I am against an implementation of the ID as a members attribute.

In my opinion you could add it as a property to the member class. But it's up to @goverfl0w whether he wants that property there or not.

@Toricane
Copy link
Contributor Author

Toricane commented Feb 4, 2022

Okay, I'll wait for fl0w

@i0bs
Copy link
Contributor

i0bs commented Feb 4, 2022

I agree with @EdVraz , I'd personally like to see that as a property method instead. The attributes per model are meant to really respect the API schema.

@Toricane
Copy link
Contributor Author

Toricane commented Feb 4, 2022

Ok I will change it to a property later today

@Toricane Toricane requested a review from EdVraz February 5, 2022 02:52
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.

LGTM

@EdVraz EdVraz merged commit e8a8e24 into interactions-py:unstable Feb 5, 2022
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.

None yet

3 participants