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

Make LTIBase.is_role() an instance method #93

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yuttie
Copy link

@yuttie yuttie commented Oct 8, 2019

What are the relevant tickets?

None.

What does this PR do?

This PR proposes to remove @staticmethod from the definition of is_role() of LTIBase class.

How should this be manually tested?

We can check if the call lti.is_role("role-name-here") is possible or not.

Where should the reviewer start?

https://github.com/yuttie/pylti/blob/make-is_role-instance-method/pylti/common.py#L549

Any background context you want to provide?

Since the signature of the method includes self, I believe this method was mistakenly marked as a static method.

@andrT
Copy link

andrT commented Apr 25, 2020

I would also like to see this pull request merged in.

@ttambet
Copy link

ttambet commented Apr 25, 2020

It looks like the mistake got in with commit ffd3d1d for !84 -- it was a real static method before, but self was added as part of that code move and @staticmethod didn't get removed.

Also travis fails, tests are broken. Maybe not so easy to fix because API change?

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.

3 participants