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

[15.0][MIG] auth_jwt: Migration to 15.0 #316

Closed
wants to merge 4 commits into from

Conversation

yankinmax
Copy link
Contributor

No description provided.

@yankinmax
Copy link
Contributor Author

@sbidoul can we proceed here?

@yankinmax yankinmax force-pushed the 15.0-mig-auth_jwt branch 2 times, most recently from eb5fcda to d7b4c03 Compare December 15, 2021 16:28
@sbidoul
Copy link
Member

sbidoul commented Dec 15, 2021

Oh, and same for the re-licensing to LGPL. I agree to do it but this should be in a separate commit or PR for easier backporting - this is not a migration commit.

@sbidoul
Copy link
Member

sbidoul commented Dec 15, 2021

Ah and can you migrate auth_jwt_demo in the same PR so we have all the tests at once ?

@yankinmax
Copy link
Contributor Author

yankinmax commented Dec 16, 2021

@sbidoul having error:

eslint...................................................................Failed
- hook id: eslint
- duration: 0.32s
- exit code: 1


odoo/external-src/server-auth/auth_jwt_demo/tests/spa/js/app.js
  1:1  error  Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

odoo/external-src/server-auth/auth_jwt_demo/tests/spa/js/oidc-client.js
  5:1  error  Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

✖ 2 problems (2 errors, 0 warnings)

Tried to change ecmaVersion: 2020 and es2020: true. Didn't help. Any idea?
Updated: same on pre-commit check.

@sbidoul
Copy link
Member

sbidoul commented Dec 16, 2021

@yankinmax could you try renaming app.js to app.esm.js (also in index.html)

@yankinmax yankinmax force-pushed the 15.0-mig-auth_jwt branch 2 times, most recently from ed55cc5 to b0c954e Compare December 20, 2021 08:25
@yankinmax
Copy link
Contributor Author

@sbidoul pre-commit check is twice canceled. What can we do here to continue?

@yankinmax
Copy link
Contributor Author

@simahawk can you take a look pls here?

auth_jwt/models/ir_http.py Outdated Show resolved Hide resolved
auth_jwt/models/ir_http.py Outdated Show resolved Hide resolved
@simahawk
Copy link
Contributor

@yankinmax what I would do:

  1. move license change to v14, merge there quickly and get back to 15
  2. open a PR w/ the module set as uninstallable for 15 and merge quickly
  3. rebase this one or open a new one for the migration

While doing so, use separated commits for improvements like #316 (comment)
This way reviewing the migration will be much easier and back porting of improvements too.

@simahawk
Copy link
Contributor

license updated on v14 here #319

@yankinmax
Copy link
Contributor Author

@sbidoul can we proceed here?

@sbidoul
Copy link
Member

sbidoul commented Jan 17, 2022

I'm confused. What is the difference between #320 and this one ?

@yankinmax
Copy link
Contributor Author

@sbidoul #320 is needed to port auth_jwt and auth_jwt_demo from v14 to v15 without any changes.
Then, when #320 will be merged I'll rebase this one and do only two commits for migration.
This helps us to avoid not related to migration commits (all these mess during review).
So, ATM pls merge #320

@sbidoul
Copy link
Member

sbidoul commented Jan 17, 2022

/ocabot migration auth_jwt

@sbidoul
Copy link
Member

sbidoul commented Jan 17, 2022

/ocabot migration auth_jwt_demo

@OCA-git-bot OCA-git-bot modified the milestone: 15.0 Jan 17, 2022
@OCA-git-bot OCA-git-bot mentioned this pull request Jan 17, 2022
18 tasks
@yankinmax
Copy link
Contributor Author

@sbidoul I rebased this PR. Get too many eslint errors on pre-commit. Should we fix them all?

@lmignon
Copy link
Sponsor Contributor

lmignon commented Jan 25, 2022

@yankinmax IMO we should ignore the file auth_jwt_demo/tests/spa/js/oidc-client.esm.js into the pre-commit checks by modifying the .pre-commit-config.yaml file.

- repo: https://github.com/pre-commit/mirrors-eslint
    rev: v7.32.0
    hooks:
      - id: eslint
        verbose: true
        args:
          - --color
          - --fix
        exclude: auth_jwt_demo/tests/spa/js/oidc-client.esm.js

@yankinmax
Copy link
Contributor Author

@yankinmax IMO we should ignore the file auth_jwt_demo/tests/spa/js/oidc-client.esm.js into the pre-commit checks by modifying the .pre-commit-config.yaml file.

- repo: https://github.com/pre-commit/mirrors-eslint
    rev: v7.32.0
    hooks:
      - id: eslint
        verbose: true
        args:
          - --color
          - --fix
        exclude: auth_jwt_demo/tests/spa/js/oidc-client.esm.js

let's try like this to speed up migrations 😃

@sbidoul
Copy link
Member

sbidoul commented Jan 25, 2022

There is already a // prettier-ignore directive in that file. I don't know why eslint complains and did not in v13, but I'd rather have a directive in the file that an exclude in the pre-commit-config, since we strive a getting the pre-commit-config identical in all repos for easier maintenance.

@@ -1,4 +1,5 @@
import {Oidc} from "./oidc-client.js";
/* eslint-disable */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this doesn't work here, any ideas?
I tried also:
/* eslint-disable-line */
/* eslint-disable-next-line */

@github-actions
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jul 10, 2022
@simahawk
Copy link
Contributor

@yankinmax any update?

@yankinmax
Copy link
Contributor Author

@simahawk I couldn't solve pre-commit issues.
#316 (comment)

@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jul 17, 2022
@github-actions
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Nov 20, 2022
@sbidoul sbidoul removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Nov 20, 2022
@github-actions
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Mar 26, 2023
@github-actions github-actions bot closed this Apr 30, 2023
@sbidoul sbidoul reopened this May 1, 2023
@github-actions github-actions bot closed this Jun 4, 2023
@gurneyalex gurneyalex reopened this Mar 1, 2024
@gurneyalex gurneyalex added no stale Use this label to prevent the automated stale action from closing this PR/Issue. and removed stale PR/Issue without recent activity, it'll be soon closed automatically. labels Mar 1, 2024
@yankinmax
Copy link
Contributor Author

Superseded by:

@yankinmax yankinmax closed this Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no stale Use this label to prevent the automated stale action from closing this PR/Issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants