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.

Sign up for GitHub

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

Jump to bottom

Require tweak #280

Merged
merged 3 commits into from
May 14, 2020
Merged

Require tweak #280

merged 3 commits into from
May 14, 2020

Conversation

psafont
Copy link
Contributor

@psafont psafont commented Jun 28, 2017

Goes ahead with the proposal in #279

Added a test and documentation on how to use the new parameter.
I took the liberty to deprecate the 'verify_xxx' options while still allowing them (as evidenced by the existing tests)

Feel free to critique any of the changes.

@coveralls
Copy link

coveralls commented Jun 28, 2017 •
edited

Coverage Status

Coverage remained the same at 100.0% when pulling 970acd8 on psafont:require-tweak into 74399b1 on jpadilla:master.

@psafont psafont force-pushed the require-tweak branch 2 times, most recently from 679938c to 4fa58b2 Compare July 12, 2017 09:32
@coveralls
Copy link

coveralls commented Jul 12, 2017 •
edited

Coverage Status

Coverage remained the same at 100.0% when pulling 4fa58b2 on psafont:require-tweak into 74399b1 on jpadilla:master.

@coveralls
Copy link

coveralls commented Jul 12, 2017 •
edited

Coverage Status

Coverage remained the same at 100.0% when pulling 4fa58b2 on psafont:require-tweak into 74399b1 on jpadilla:master.

@psafont psafont mentioned this pull request Jul 12, 2017
Closed
jpadilla
jpadilla requested changes Oct 19, 2017
View reviewed changes
Copy link
Owner

@jpadilla jpadilla left a comment

Choose a reason for hiding this comment

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

Yep, I think we can do this.

@mark-adams thoughts?

jwt/api_jwt.py Outdated Show resolved Hide resolved
jwt/api_jwt.py Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Oct 19, 2017 •
edited

Coverage Status

Coverage increased (+0.002%) to 99.705% when pulling 5f7be20 on psafont:require-tweak into f67d112 on jpadilla:master.

@psafont
Copy link
Contributor Author

psafont commented Oct 19, 2017

Don't know why it's failing, hopefully I didn't mess the rebase.

@jpadilla
Copy link
Owner

@psafont not to worry, that should be fixed once #297 is taken care of.

@jpadilla
Copy link
Owner

@psafont mind rebasing from master? #297 is now solved.

@coveralls
Copy link

coveralls commented Oct 20, 2017 •
edited

Coverage Status

Coverage remained the same at 100.0% when pulling b894d78 on psafont:require-tweak into e1e4d02 on jpadilla:master.

@psafont
Copy link
Contributor Author

psafont commented Oct 23, 2017

Just in case it was missed, I've rebased it again.

mark-adams
mark-adams suggested changes Nov 25, 2017
View reviewed changes
Copy link
Contributor

@mark-adams mark-adams left a comment

Choose a reason for hiding this comment

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

A couple minor changes

jwt/api_jwt.py Outdated Show resolved Hide resolved
jwt/api_jwt.py Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Nov 30, 2017 •
edited

Pull Request Test Coverage Report for Build 118

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at ?%
Totals Coverage Status
Change from base Build 114: 0.0%
Covered Lines:
Relevant Lines: 0
💛 - Coveralls

@psafont
Copy link
Contributor Author

psafont commented Jun 14, 2018

@mark-adams It's been quite a while since the last changes, is something else needed to accept these changes?

This functionality would still simplify my code :)

@mathiasose
Copy link

Bump. I would also like to see this functionality released.

@dave-shawley dave-shawley mentioned this pull request Nov 24, 2019
Closed
@psafont
Copy link
Contributor Author

psafont commented Nov 26, 2019

Rebased patchset to master. Is there anything missing?

auvipy
auvipy approved these changes May 14, 2020
View reviewed changes
Copy link
Collaborator

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

Looks good.

@auvipy auvipy merged commit 008490a into jpadilla:master May 14, 2020
@auvipy
Copy link
Collaborator

auvipy commented May 14, 2020

thanks for your patience!

@psafont psafont deleted the require-tweak branch May 14, 2020 15:07
jpadilla pushed a commit that referenced this pull request Jun 19, 2020
@psafont @jpadilla
Require tweak (#280)
d8474f9 
* Use require options as a list, instead of booleans

Deprecate the use of the boolean options

* Add test for the new require option

* Add documentation on how to use the require option

Co-authored-by: Pau Ruiz i Safont <psafont@ebi.ac.uk>
@salcedo salcedo mentioned this pull request Jul 28, 2020
Closed
@jpadilla jpadilla added this to the v2.0.0 milestone Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mark-adams mark-adams mark-adams requested changes

@jpadilla jpadilla jpadilla approved these changes

@auvipy auvipy auvipy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.0.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@psafont @coveralls @jpadilla @mathiasose @auvipy @mark-adams