-
Notifications
You must be signed in to change notification settings - Fork 104
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
[16.0][ADD] shopinvader_api_signin_jwt #1482
[16.0][ADD] shopinvader_api_signin_jwt #1482
Conversation
3751b38
to
0a10f08
Compare
|
||
@signin_router.post("/signin", status_code=200) | ||
def signin( | ||
# TODO use fastai_auth_jwt.auth_jwt_authenticated_odoo_env when it is fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qgroulard Can you elaborate on what's the problem? There is no reported issue...
To move forward you could moves the TODOs into a ROADMAP.rst file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just opened OCA/rest-framework#406.
And TODOs have been moved to the roadmap 👍
0a10f08
to
9526b84
Compare
/signout looks good to me. 👍 |
7509fa0
to
ac4f333
Compare
ac4f333
to
77fc96a
Compare
This one is correct as is. Roadmap items can be addressed in followup PRs. /ocabot merge nobump |
On my way to merge this fine PR! |
@sbidoul your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-1482-by-sbidoul-bump-nobump. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
Looks like the main branch is red 😢 |
Retrying /ocabot merge patch |
This PR looks fantastic, let's merge it! |
Congratulations, your PR was merged at d22372e. Thanks a lot for contributing to shopinvader. ❤️ |
Possible improvements: