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

Extension on Reaction events to return Member #351

Merged
merged 5 commits into from
Aug 4, 2017
Merged

Extension on Reaction events to return Member #351

merged 5 commits into from
Aug 4, 2017

Conversation

Daniel-Worrall
Copy link
Contributor

@Daniel-Worrall Daniel-Worrall commented May 29, 2017

  • Specs Won't add yet

@user = @bot.user(@user_id).on(@server)
else
@user = @bot.user(@user_id)
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this could be neatened to:

@user ||= if server
            server.member(@user_id)
          else
            @bot.user(@user_id)
          end

@Daniel-Worrall
Copy link
Contributor Author

This is good to merge. I'm abstaining on the specs for now due to thinking about a better method of providing spec fixtures than the structure we have right now. Tested with eval code

def user
# Cache the user so we don't do requests all the time
@user ||= @bot.user(@user_id)
@user ||= if server
@bot.user(@user_id).on(@server)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be simplified to @server.member(@user_id)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I told him to do that but Nekka doesn't like me.

@meew0 meew0 added this to the 3.3.0 milestone Aug 4, 2017
@meew0 meew0 merged commit 551fc81 into discordrb:master Aug 4, 2017
@Daniel-Worrall Daniel-Worrall deleted the reaction-server branch April 4, 2018 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants