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] storage_file: Migration to 18.0 #434

Merged
merged 107 commits into from
Feb 10, 2025

Conversation

thienvh332
Copy link
Contributor

No description provided.

Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Thanks!

@StefanRijnhart
Copy link
Member

Maybe squash 'Raph's commits into one big [WIP] storage_file commit
image

@thienvh332 thienvh332 force-pushed the 18.0-mig-storage_file_temp branch from f16bdd7 to bfa8518 Compare December 5, 2024 03:01
@thienvh332 thienvh332 force-pushed the 18.0-mig-storage_file_temp branch from bfa8518 to 5dc012d Compare December 6, 2024 02:57
Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Thanks for the updates!

if record.data:
raise UserError(
self.env._(
"File can not be updated," "remove it and create a new one"

Choose a reason for hiding this comment

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

Suggested change
"File can not be updated," "remove it and create a new one"
"File can not be updated, remove it and create a new one"

def _clean_storage_file(self):
# we must be sure that all the changes are into the DB since
# we by pass the ORM
self.env.flush_all()

Choose a reason for hiding this comment

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

Suggested change
self.env.flush_all()
self.flush_model(["to_delete"])

nitpicking: this is actually more accurate

self.env[storage_file._name].with_user(public_user).browse(
storage_file.ids
).name
_ = (

Choose a reason for hiding this comment

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

Why are you assigning to _ ?

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 is why I declared the variable unused for it: B018 Found useless attribute access. Either assign it to a variable or remove it.

Choose a reason for hiding this comment

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

Oh, if it's just a linter rule, you can disable it it with an inline comment.
E.g.:

self.env[storage_file._name].with_user(public_user).browse(storage_file.ids).name  # noqa: B018

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks you, It doesn't work because it crossed the line when combined with pre-commit with max characters being 88 per line

Copy link
Contributor

@lmignon lmignon Jan 30, 2025

Choose a reason for hiding this comment

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

@thienvh332 Can you at least assign the value to a string starting with _ to avoid the warning but not to the character _ alone to avoid the confusion with the translation method _.

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

@thienvh332 thienvh332 force-pushed the 18.0-mig-storage_file_temp branch from 5dc012d to 002bf64 Compare January 7, 2025 03:41
self.env[storage_file._name].with_user(public_user).browse(
storage_file.ids
).name
_ = (

Choose a reason for hiding this comment

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

Oh, if it's just a linter rule, you can disable it it with an inline comment.
E.g.:

self.env[storage_file._name].with_user(public_user).browse(storage_file.ids).name  # noqa: B018

@thienvh332 thienvh332 force-pushed the 18.0-mig-storage_file_temp branch 2 times, most recently from c1aaba9 to f8a649e Compare January 8, 2025 03:23
Copy link
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

Thank you for the migration. A little comment....

self.env[storage_file._name].with_user(public_user).browse(
storage_file.ids
).name
_ = (
Copy link
Contributor

@lmignon lmignon Jan 30, 2025

Choose a reason for hiding this comment

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

@thienvh332 Can you at least assign the value to a string starting with _ to avoid the warning but not to the character _ alone to avoid the confusion with the translation method _.

@lmignon
Copy link
Contributor

lmignon commented Jan 30, 2025

/ocabot migrate storage_file

@OCA-git-bot
Copy link
Contributor

Hi @lmignon. Your command failed:

Invalid command: migrate.

Ocabot commands

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

More information

@lmignon
Copy link
Contributor

lmignon commented Jan 30, 2025

/ocabot migration storage_file

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Jan 30, 2025
@OCA-git-bot OCA-git-bot mentioned this pull request Jan 30, 2025
21 tasks
@thienvh332 thienvh332 force-pushed the 18.0-mig-storage_file_temp branch from f8a649e to c3efaf7 Compare February 4, 2025 02:34
@thienvh332 thienvh332 requested a review from lmignon February 4, 2025 02:37
@simahawk
Copy link
Contributor

/ocabot merge nobump

@simahawk
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

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

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

Congratulations, your PR was merged at 9f65f0c. 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.