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

Add version 18 #272

Merged
merged 4 commits into from
Sep 30, 2024
Merged

Add version 18 #272

merged 4 commits into from
Sep 30, 2024

Conversation

SirAionTech
Copy link
Contributor

No description provided.

@pedrobaeza
Copy link
Member

The question here is also to bump the linters versions up to the recommended/compatible ones.

@SirAionTech SirAionTech marked this pull request as ready for review September 26, 2024 14:20
@SirAionTech
Copy link
Contributor Author

SirAionTech commented Sep 26, 2024

@sbidoul
Copy link
Member

sbidoul commented Sep 26, 2024

The pyyaml error in https://github.com/OCA/oca-addons-repo-template/actions/runs/11054048607/job/30709905165?pr=272#step:7:90 looks like the known issue yaml/pyyaml#601

I continue to think we should not use poetry nor pin dependencies to test this repo. cc/ @yajo

@SirAionTech
Copy link
Contributor Author

SirAionTech commented Sep 26, 2024

The question here is also to bump the linters versions up to the recommended/compatible ones.

There you go: 5cf221c now it's e613651

prettier could also be updated but:

I'd remove that for another alternative but don't know any, so I left it as it was

@SirAionTech
Copy link
Contributor Author

@sbidoul using Python 3.9 fixes https://github.com/OCA/oca-addons-repo-template/actions/runs/11054931643/job/30712926871#step:8:309

AttributeError: module 'pkgutil' has no attribute 'ImpImporter'. Did you mean: 'zipimporter'?

but breaks builds on Ubuntu 20.04 so I have removed them, CI is still requiring them because of some hidden setting I think (that should now include 17 and 18), let me know if I have to do anything

@@ -238,7 +238,7 @@ repos:
additional_dependencies: ["flake8-bugbear=={{ repo_rev.flake8_bugbear }}"]
{%- else %}
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.1.3
rev: v0.6.8
Copy link
Member

Choose a reason for hiding this comment

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

I think we should have a variable for this ruff version, so we can control which one we use with each Odoo version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is ruff particular? Otherwise I'd fall back to #272 (comment):

I could have left everything < 18 crystallized as it was but maybe they should be updated too from time to time, don't they?

{%- set repo_rev.prettier = "2.7.1" %}
{%- set repo_rev.prettier_xml = "2.2.0" %}
{%- set repo_rev.pylint_odoo = "v8.0.19" %}
{%- set repo_rev.pylint_odoo = "v9.1.2" %}
Copy link
Member

Choose a reason for hiding this comment

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

Is it safe to upgrade all linters for 16 and 17 too? What is the risk of making existing repos red due to new checks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it safe to upgrade all linters for 16 and 17 too?

Is it safe to keep using the old ones? For how long should we keep using them?
I'm for using the new and shiny things, however I'll change it if you think it's better.

What is the risk of making existing repos red due to new checks?

I'll try the update in a repo to see how much it should change to satisfy the new requirements.

I could have left everything < 18 crystallized as it was but maybe they should be updated too from time to time, don't they?

Copy link
Member

Choose a reason for hiding this comment

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

I could have left everything < 18 crystallized as it was but maybe they should be updated too from time to time, don't they?

I don't think we should update for previous branches unless necessary. Otherwise tiny changes in linters will make existing branches red, and this is really annoying.

So I would add a new section for 18, with the latest linters.

Copy link
Member

Choose a reason for hiding this comment

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

Old branches won't be affected unless manually updated. And in that case, pre-commit will update things and maybe ask for new changes, but that can be considered as part of that same manual work. However, it's true that if there must be a mass update of non-pre-commit-related things, that can be problematic. Oftentimes you're the one who does that, @sbidoul, so as you wish.

prettier 3.x with prettier-xml plugin is affected by prettier/prettier#15696
Ubuntu 20.04 uses Python3.8
@sbidoul
Copy link
Member

sbidoul commented Sep 30, 2024

Let's try this. Thanks a lot @SirAionTech !

@sbidoul sbidoul merged commit cfe36d6 into OCA:master Sep 30, 2024
7 checks passed
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.

5 participants