Skip to content

Fixing the status codes #80

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

Merged
merged 4 commits into from
Dec 14, 2020
Merged

Fixing the status codes #80

merged 4 commits into from
Dec 14, 2020

Conversation

creyD
Copy link

@creyD creyD commented Dec 10, 2020

As I said in #79 I adjusted the status codes to return what is documented.

@fitodic
Copy link
Collaborator

fitodic commented Dec 14, 2020

@creyD Thanks for the PR 🙂 Would you mind adding a changelog and updating the returned status code in the test suite?

@creyD
Copy link
Author

creyD commented Dec 14, 2020

@fitodic Sure! Sorry I didn't see the Contributing.md before.

@creyD
Copy link
Author

creyD commented Dec 14, 2020

Can you test it for you again? There was one test failing, but i don't think this is because of my changes. test_authentication.py Line 120 for reference.

@fitodic
Copy link
Collaborator

fitodic commented Dec 14, 2020

Can you test it for you again? There was one test failing, but i don't think this is because of my changes. test_authentication.py Line 120 for reference.

If that's the one regarding sets and payload key's, it's already been fixed in #81. The only thing that remains is to pull the master branch and push the changes 🙂

UPDATE: I see the latest pipeline has passed so I guess that's it.

@fitodic fitodic merged commit 623265f into Styria-Digital:master Dec 14, 2020
@creyD
Copy link
Author

creyD commented Dec 14, 2020

Thanks for merging this so fast! When will there be a new release?

@fitodic
Copy link
Collaborator

fitodic commented Dec 14, 2020

Thanks for merging this so fast! When will there be a new release?

1.17.3 is available on PyPI 🙂

@creyD
Copy link
Author

creyD commented Dec 14, 2020

Thanks!

@dominik-bln
Copy link

Quick Feedback: This change was a bit surprising in a patch update because it alters the behaviour of the API, i. e. it is a breaking change. For us it hopefully only broke the tests, but changing the docs or putting this in a major update would have seemed more appropriate.

@marcosox
Copy link

Agree, it's a breaking change which should have gone in a major version update. Thankfully I read changelogs before updating :)

decibyte added a commit to decibyte/django-rest-framework-jwt that referenced this pull request Oct 9, 2024
It looks like updating the documentation was forgotten as part of Styria-Digital#80.
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.

4 participants