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.

Sign up for GitHub

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

Jump to bottom

[18.0][MIG] sql_request_abstract: Migration to 18.0 #937

Merged
merged 91 commits into from
Nov 12, 2024
Merged

[18.0][MIG] sql_request_abstract: Migration to 18.0 #937

merged 91 commits into from
Nov 12, 2024
+21,762 −0

Conversation

thienvh332
Copy link

@thienvh332 thienvh332 marked this pull request as draft October 10, 2024 04:05
@thienvh332 thienvh332 marked this pull request as ready for review October 10, 2024 04:10
@thienvh332 thienvh332 mentioned this pull request Oct 10, 2024
Open
@legalsylvain
Copy link
Contributor

hi. could you take a look on red CI ? thanks !

@thienvh332
Copy link
Author

Hi @legalsylvain ,
I updated it. Could you have a look?

legalsylvain
legalsylvain reviewed Oct 11, 2024
View reviewed changes
checklog-odoo.cfg
@@ -0,0 +1,3 @@
[checklog-odoo]
ignore=
WARNING.*0 failed, 0 error\(s\).*
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this addition ?

Copy link
Author

@thienvh332 thienvh332 Oct 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This module originally had no test case and the new oca-ci test case was raising errors even with WARNING. So I added it to avoid being labeled as ERROR

Copy link
Contributor

Choose a reason for hiding this comment

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

hum. something is wrong if CI is red if there are no test, it is a valid use case. I think that something should be fixed in checklog-odoo tools instead.

@sbidoul : what do you think ?

Copy link
Member

Choose a reason for hiding this comment

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

something should be fixed in checklog-odoo tools instead.

We don't want to special case this in checklog-odoo, because we have had situations where for some reason tests did not run had that error was actually helpful.

This problem will only occur when the first module migrated has no test, so in practice this should not occur often, I guess.

rvalyi
rvalyi approved these changes Oct 24, 2024
View reviewed changes
Copy link
Member

@rvalyi rvalyi left a comment

Choose a reason for hiding this comment

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

Ok. Now this was a pretty trivial migration, not sure it is fair to be listed in the contributors and put other credits. Let the module authors be the judges...

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@HviorForgeFlow
Copy link
Member

trivial or not, an effort to move this module to v18 has been done. Commit history will put everybody in their place. Let's merge it :)

@HviorForgeFlow
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 18.0-ocabot-merge-pr-937-by-HviorForgeFlow-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Nov 8, 2024
@OCA-git-bot
Merge PR #937 into 18.0
ca17829 
Signed-off-by HviorForgeFlow
@OCA-git-bot
Copy link
Contributor

@HviorForgeFlow your merge command was aborted due to failed check(s), which you can inspect on this commit of 18.0-ocabot-merge-pr-937-by-HviorForgeFlow-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.

@OCA-git-bot
Copy link
Contributor

@HviorForgeFlow your merge command was aborted due to failed check(s), which you can inspect on this commit of 18.0-ocabot-merge-pr-937-by-HviorForgeFlow-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.

@HviorForgeFlow
Copy link
Member

@thienvh332 pre-commit failed

weblate and others added 22 commits November 12, 2024 09:28
@weblate @thienvh332
Update translation files
693ad0f 
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: reporting-engine-16.0/reporting-engine-16.0-sql_request_abstract
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-sql_request_abstract/
@mymage @thienvh332
Translated using Weblate (Italian)
d4ff237 
Currently translated at 14.2% (6 of 42 strings)

Translation: reporting-engine-16.0/reporting-engine-16.0-sql_request_abstract
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-sql_request_abstract/it/
@nguyenminhchien @thienvh332
[IMP] sql_request_abstract: pre-commit auto fixes
8b5b181
@nguyenminhchien @thienvh332
[MIG] sql_request_abstract: Migration to 17.0
b125c4a
@thienvh332
[UPD] Update sql_request_abstract.pot
e5d9ffe
@OCA-git-bot @thienvh332
[BOT] post-merge updates
e232423
@weblate @thienvh332
Update translation files
0f82742 
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: reporting-engine-17.0/reporting-engine-17.0-sql_request_abstract
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-17-0/reporting-engine-17-0-sql_request_abstract/
@weblate @thienvh332
Update translation files
27e8fe8 
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: reporting-engine-17.0/reporting-engine-17.0-sql_request_abstract
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-17-0/reporting-engine-17-0-sql_request_abstract/
@Ivorra78 @thienvh332
Translated using Weblate (Spanish)
c72f4ac 
Currently translated at 95.4% (42 of 44 strings)

Translation: reporting-engine-17.0/reporting-engine-17.0-sql_request_abstract
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-17-0/reporting-engine-17-0-sql_request_abstract/es/
@Ivorra78 @thienvh332
Translated using Weblate (Spanish)
5999c6c 
Currently translated at 100.0% (44 of 44 strings)

Translation: reporting-engine-17.0/reporting-engine-17.0-sql_request_abstract
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-17-0/reporting-engine-17-0-sql_request_abstract/es/
@mymage @thienvh332
Translated using Weblate (Italian)
0375996 
Currently translated at 100.0% (44 of 44 strings)

Translation: reporting-engine-17.0/reporting-engine-17.0-sql_request_abstract
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-17-0/reporting-engine-17-0-sql_request_abstract/it/
@jakobkrabbe @thienvh332
Translated using Weblate (Swedish)
fc05f9d 
Currently translated at 100.0% (44 of 44 strings)

Translation: reporting-engine-17.0/reporting-engine-17.0-sql_request_abstract
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-17-0/reporting-engine-17-0-sql_request_abstract/sv/
@thienvh332
[UPD] Update sql_request_abstract.pot
728568b
@weblate @thienvh332
Update translation files
2b0a1a7 
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: reporting-engine-17.0/reporting-engine-17.0-sql_request_abstract
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-17-0/reporting-engine-17-0-sql_request_abstract/
@thienvh332
[UPD] Update sql_request_abstract.pot
247486a
@weblate @thienvh332
Update translation files
54d20f5 
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: reporting-engine-17.0/reporting-engine-17.0-sql_request_abstract
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-17-0/reporting-engine-17-0-sql_request_abstract/
@ibuioli @thienvh332
Translated using Weblate (Spanish (Argentina))
464033c 
Currently translated at 100.0% (43 of 43 strings)

Translation: reporting-engine-17.0/reporting-engine-17.0-sql_request_abstract
Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-17-0/reporting-engine-17-0-sql_request_abstract/es_AR/
@florian-dacosta @thienvh332
[IMP] Make possible to make sql queries on an external database
154f376
@thomaspaulb @thienvh332
[FIX] sql_request_abstract: regression
1f0844c
@thienvh332
[IMP] sql_request_abstract: apply pre-commit auto fixes
0b59e05
@thienvh332
[MIG] sql_request_abstract: Migration to 18.0
a1f169c
@thienvh332
add checklog-odoo.cfg
78b3649
@legalsylvain
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 18.0-ocabot-merge-pr-937-by-legalsylvain-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 90791b7 into OCA:18.0 Nov 12, 2024
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at b0dfe5d. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbidoul sbidoul sbidoul left review comments

@sebalix sebalix sebalix approved these changes

@legalsylvain legalsylvain legalsylvain approved these changes

@rvalyi rvalyi rvalyi approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.