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

[18.0][MIG] stock_dock: Migration to 18.0 #160

Merged
merged 20 commits into from
Feb 3, 2025

Conversation

Kimkhoi3010
Copy link

@Kimkhoi3010 Kimkhoi3010 commented Jan 8, 2025

I found only 1 commit to port but it was ignored, I added it to blacklist:

During the migration of the stock_dock module, I identified two issues in the tms module that needed to be addressed. To resolve these issues, I created two separate commits:

  • Commit 1: The kanban-box in the tms module was deprecated in Odoo. This commit replaces the kanban-box definition with the recommended card template to address the warning.
  • Commit 2: The group_tms_uom field in the tms module had the same label as group_uom from the product module ("Units of Measure"), causing a conflict. This commit updates the label of group_tms_uom to ensure uniqueness.

@Kimkhoi3010 Kimkhoi3010 marked this pull request as draft January 8, 2025 03:06
@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-stock_dock branch 4 times, most recently from c7a96bd to ca9252f Compare January 8, 2025 04:03
@Kimkhoi3010 Kimkhoi3010 marked this pull request as ready for review January 8, 2025 04:16
@rousseldenis
Copy link
Contributor

/ocabot migration stock_dock

@rousseldenis
Copy link
Contributor

rousseldenis commented Jan 8, 2025

@Kimkhoi3010 Thanks for this. Could you remove all changes related to other modules?

You should respect flow for migration:

https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-18.0#technical-method-to-migrate-a-module-from-170-to-180-branch

tms changes should go in another PR.

@Kimkhoi3010 Kimkhoi3010 marked this pull request as draft January 8, 2025 08:40
@Kimkhoi3010 Kimkhoi3010 force-pushed the 18.0-mig-stock_dock branch 3 times, most recently from bd944e8 to ea0867c Compare January 15, 2025 07:46
@Kimkhoi3010
Copy link
Author

Kimkhoi3010 commented Jan 15, 2025

@Kimkhoi3010 Thanks for this. Could you remove all changes related to other modules?

You should respect flow for migration:

https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-18.0#technical-method-to-migrate-a-module-from-170-to-180-branch

tms changes should go in another PR.

Hi @rousseldenis
I updated by your comment, I have created a PR to fix the issues in tms module: #164

@Kimkhoi3010 Kimkhoi3010 marked this pull request as ready for review January 15, 2025 07:49
@rousseldenis
Copy link
Contributor

indeed, adding at least some test cases is better. But strangely, right now, no checklog_odoo.cfg still test builds are passed :)

Because tms module has been merged

@Kimkhoi3010
Copy link
Author

Kimkhoi3010 commented Jan 24, 2025

I'm unsure about the necessity of the checklog_odoo.cfg file. @Kimkhoi3010, could you please explain its inclusion? I'm not aware of any new standards introduced for OCA.

@Kimkhoi3010 @bizzappdev That's because on void tests repository main branch, no tests are run. And no tests are declared in this one.

Even if adding that line to configuration solves errors, I always recommend to add a basic test.

@Kimkhoi3010 Here, you can add a test that checks the default warehouse.

H

I'm unsure about the necessity of the checklog_odoo.cfg file. @Kimkhoi3010, could you please explain its inclusion? I'm not aware of any new standards introduced for OCA.

@Kimkhoi3010 @bizzappdev That's because on void tests repository main branch, no tests are run. And no tests are declared in this one.

Even if adding that line to configuration solves errors, I always recommend to add a basic test.

@Kimkhoi3010 Here, you can add a test that checks the default warehouse.

Hi @rousseldenis, @bizzappdev
I have updated according to your comment, I have added a basic test

Copy link

@bizzappdev bizzappdev left a comment

Choose a reason for hiding this comment

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

Code and Functional Testing LGTM

@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). 🤖

@simahawk
Copy link

@Kimkhoi3010 pls rewrite the last commit as [imp] stock_dock: add tests

@Kimkhoi3010
Copy link
Author

Kimkhoi3010 commented Feb 3, 2025

@Kimkhoi3010 pls rewrite the last commit as [imp] stock_dock: add tests

Hi @simahawk,
I updated this PR by your comment. Thanks for your time.

@simahawk
Copy link

simahawk commented Feb 3, 2025

@Kimkhoi3010 pls rewrite the last commit as [imp] stock_dock: add tests

Hi @simahawk, I updated this PR by your comment. Thanks for your time.

I still don't see the module name, there's a not so useful - instead 😉

@Kimkhoi3010
Copy link
Author

@Kimkhoi3010 pls rewrite the last commit as [imp] stock_dock: add tests

Hi @simahawk, I updated this PR by your comment. Thanks for your time.

I still don't see the module name, there's a not so useful - instead 😉

Thanks for your time.

@simahawk
Copy link

simahawk commented Feb 3, 2025

/ocabot merge nobumo

@OCA-git-bot
Copy link
Contributor

Hi @simahawk. Your command failed:

Invalid options for command merge: nobumo.

Ocabot commands

  • ocabot merge major|minor|patch|nobump
  • ocabot rebase
  • ocabot migration {MODULE_NAME}

More information

@simahawk
Copy link

simahawk commented Feb 3, 2025

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-160-by-simahawk-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit a31d3ff into OCA:18.0 Feb 3, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 6bb49bf. 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.