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

[14.][UPD] Atualização das bibliotecas do pre-commit #2747

Closed
wants to merge 1 commit into from

Conversation

mbcosta
Copy link
Contributor

@mbcosta mbcosta commented Oct 25, 2023

Update Pre-Commit librarys.

Atualizando as bibliotecas do pre-commit, ao tentar resolver um Warning [W7936(missing-manifest-dependency), ] Missing external dependency #2744 acabei percebendo que isso ocorre porque as bibliotecas do pre-commit estão desatualizadas, ao atualizar esse Warning deixa de aparecer, porém isso acaba alterando diversos outros arquivos, as alterações são simples como remoção de linhas vazias, método super sem parâmetros, nome de variável não pode ser uma letra L e etc, para evitar uma revisão extensa e tentar facilitar vou criar novos PRs em separado corrigindo esses problemas, por isso aqui por enquanto vai ter erro.

cc @rvalyi @renatonlima @marcelsavegnago @mileo

@mileo
Copy link
Member

mileo commented Oct 25, 2023

@mbcosta o pre-commit falhou.

@rvalyi
Copy link
Member

rvalyi commented Oct 25, 2023

@mbcosta como vc pegou essas versões? A principio basta rodar copier update para atualizar o arquivo de .pre-commit.yaml (e os outros arquivos meta). Foi o que eu fiz neste outro PR #2748
Sera se ele não resolve?

@mbcosta
Copy link
Contributor Author

mbcosta commented Oct 25, 2023

@mileo era esperado como esta alterando diversos arquivos eu vou ver de fazer PRs em separado para facilitar revisão "Remoção de Linhas Vazias", "Remoção de Parametros do Super"

@rvalyi eu acessei os links dos arquivos olhei a última versão e inclui no arquivo, no PR que vocês fez parece que não houve a atualização PyLint https://github.com/OCA/pylint-odoo para a rev: v9.0.0 que é o que está gerando um Warning desnecessário Warning [W7936(missing-manifest-dependency), ] Missing external dependency as outras libs com Flake8 também não está sendo atualizada.

@rvalyi
Copy link
Member

rvalyi commented Oct 25, 2023

eu não analisei muito bem, mas geralmente numa série do Odoo não vai se usar a última versão das ferramentas pois geralmente vão ser utilizadas apenas em séries do Odoo mais novas como v15 ou v16. Pode ser que a gente pode abrir uma exceção para alguma ferramenta mas a princípio a forma de atualizar seria pelo copier mesmo...

@mbcosta mbcosta force-pushed the 14.0-UPD-bibliotecas_pre_commit branch from 575be71 to 623b17b Compare November 25, 2023 18:58
@mbcosta
Copy link
Contributor Author

mbcosta commented Nov 25, 2023

Para ficar verde está dependendo do PR #2753 os erros podem ser vistos aqui https://github.com/OCA/l10n-brazil/actions/runs/6990853819/job/19020782456?pr=2747#step:7:171

Como o @rvalyi apontou idealmente o pre-commit deveria ser atualizado apenas com o uso do comando "copier update" https://github.com/OCA/oca-addons-repo-template/ cada versão do odoo tem uma lista de pacotes em uma determinada versão https://github.com/OCA/oca-addons-repo-template/blob/master/src/.pre-commit-config.yaml.jinja#L5 , então porque fazer essa alteração manual?

Atualizando o pre-commit foi possível localizar um problema de IDs duplicados nos CSV bcada24 e as alterações que passaram a ser feitas são as mesmas que já estão sendo vistas na v17, foi feito um PR de teste no mis-builder ainda na v16 https://github.com/OCA/mis-builder/pull/577/files onde é possível ver algumas das correções que também foram necessárias aqui

Método Sem Parâmetro #2753
image

Linhas Vazias #2749
image

Então essa alteração pode deixar mais simples tanto a migração quando os PortFowards ou BackForwards, já que esses erros de pre-commit passam a não acontecer entre as versões.

  • Contra
    Como é uma alteração manual sempre que for necessário rodar o "copier update" a versão das bibliotecas vai precisar ser vista, mas hoje acho que na v14 isso já está praticamente "congelado" e não deve ter necessidade de rodar o comando nos próximos meses ou mesmo novamente nessa versão, talvez tenha outros motivos contra, o CI em geral não parece ter sido afetado.

Uma outra possibilidade e manter esse PR em Aberto e fazer o rebase de tempos em tempos para localizar os erros de pre-commit entre as versões e assim diminuir esse problema.

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 21, 2024
@mileo
Copy link
Member

mileo commented Jul 30, 2024

Podemos fechar esse?

@rvalyi
Copy link
Member

rvalyi commented Jul 30, 2024

eu diria que sim

@rvalyi rvalyi closed this Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants