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

[14.0][FIX] account_fiscal_year_closing, migration script for fiscal year multi company rule #275

Merged
merged 1 commit into from
Oct 31, 2023
Merged

[14.0][FIX] account_fiscal_year_closing, migration script for fiscal year multi company rule #275

merged 1 commit into from
Oct 31, 2023

Conversation

GSLabIt
Copy link
Contributor

@GSLabIt GSLabIt commented Oct 27, 2023 •
edited
Loading

migration script for fiscal year multi company rule, fix noupdate changes on installed modules

@GSLabIt GSLabIt force-pushed the 14.0-account_fiscal_year_closing-migration_script branch 5 times, most recently from 2203e8b to 8f7b66d Compare October 27, 2023 09:08
@GSLabIt GSLabIt changed the title [FIX] migration script for fiscal year multi company rule [14.0][FIX] account_fiscal_year_closing, migration script for fiscal year multi company rule Oct 27, 2023
@francesco-ooops
Copy link
Contributor

@matteoopenf since you did migration to v14, does this look good for you?

@matteoopenf
Copy link
Contributor

@matteoopenf since you did migration to v14, does this look good for you?

I think is ok, @GSLabIt you could fix pre-commit?
After fix I will approve

@GSLabIt
Copy link
Contributor Author

GSLabIt commented Oct 27, 2023

@matteoopenf since you did migration to v14, does this look good for you?

I think is ok, @GSLabIt you could fix pre-commit? After fix I will approve

not a pr issue, it's generalized. See OCA/oca-addons-repo-template#214

@matteoopenf
Copy link
Contributor

@matteoopenf since you did migration to v14, does this look good for you?

I think is ok, @GSLabIt you could fix pre-commit? After fix I will approve

not a pr issue, it's generalized. See OCA/oca-addons-repo-template#214

ok I read the conversation we should wait the issue is resolved, when solved tag me again and I will approve, because if the precommit fail, is not possibile merge

@GSLabIt
Copy link
Contributor Author

GSLabIt commented Oct 27, 2023

@matteoopenf since you did migration to v14, does this look good for you?

I think is ok, @GSLabIt you could fix pre-commit? After fix I will approve

not a pr issue, it's generalized. See OCA/oca-addons-repo-template#214

ok I read the conversation we should wait the issue is resolved, when solved tag me again and I will approve, because if the precommit fail, is not possibile merge

you could approve anyway imho

matteoopenf
matteoopenf approved these changes Oct 27, 2023
View reviewed changes
Copy link
Contributor

@matteoopenf matteoopenf left a comment

Choose a reason for hiding this comment

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

LGTM

@matteoopenf
Copy link
Contributor

@matteoopenf since you did migration to v14, does this look good for you?

I think is ok, @GSLabIt you could fix pre-commit? After fix I will approve

not a pr issue, it's generalized. See OCA/oca-addons-repo-template#214

ok I read the conversation we should wait the issue is resolved, when solved tag me again and I will approve, because if the precommit fail, is not possibile merge

you could approve anyway imho

done

@pedrobaeza
Copy link
Member

If you are in a hurry, you can apply manually the patch in the repo in a different PR, like this one:

OCA/sale-workflow#2748

@GSLabIt
Copy link
Contributor Author

GSLabIt commented Oct 27, 2023

If you are in a hurry, you can apply manually the patch in the repo in a different PR, like this one:

OCA/sale-workflow#2748

Not in a hurry, but thanks for the hint

sergiocorato
sergiocorato reviewed Oct 30, 2023
View reviewed changes
Copy link
Contributor

@sergiocorato sergiocorato left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, I added some comments.

account_fiscal_year_closing/views/account_fiscalyear_closing_template_views.xml Outdated Show resolved Hide resolved
account_fiscal_year_closing/views/account_fiscalyear_closing_template_views.xml Outdated Show resolved Hide resolved
@GSLabIt GSLabIt force-pushed the 14.0-account_fiscal_year_closing-migration_script branch 2 times, most recently from 8ece6c5 to f910645 Compare October 30, 2023 15:48
sergiocorato
sergiocorato approved these changes Oct 30, 2023
View reviewed changes
Copy link
Contributor

@sergiocorato sergiocorato left a comment

Choose a reason for hiding this comment

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

LGTM

@sergiocorato
Copy link
Contributor

I'll merge this PR even if pre-commit is failed, as #276 is blocked.

@francesco-ooops
Copy link
Contributor

@sergiocorato can you make a PR to fix dotfiles as done for example in OCA/purchase-workflow#2075 or OCA/l10n-italy#3687 ?

We're working on fixing other repos. Thanks!

@sergiocorato
Copy link
Contributor

I'll merge this PR even if pre-commit is failed, as #276 is blocked.

It's not possible I think, @etobella please put in #276 all the pre-commit fixes.

@francesco-ooops
Copy link
Contributor

@sergiocorato can you make a PR to fix dotfiles as done for example in OCA/purchase-workflow#2075 or OCA/l10n-italy#3687 ?

We're working on fixing other repos. Thanks!

@sergiocorato you can just open a new PR with the necessary fixes as we've done in the linked PRs above ;)

@sergiocorato
Copy link
Contributor

sergiocorato commented Oct 30, 2023 •
edited
Loading

@sergiocorato can you make a PR to fix dotfiles as done for example in OCA/purchase-workflow#2075 or OCA/l10n-italy#3687 ?
We're working on fixing other repos. Thanks!

@sergiocorato you can just open a new PR with the necessary fixes as we've done in the linked PRs above ;)

#276 already does the copier template update to 1.17.2 (as done in OCA/purchase-workflow#2075 ), to be mergeable the same PR has to do all the fixes.

@francesco-ooops
Copy link
Contributor

@sergiocorato we superseded @etobella 's PRs to help him get the work done, then he merged them. Or maybe there's something I'm not grasping here

@sergiocorato
Copy link
Contributor

@sergiocorato we superseded @etobella 's PRs to help him get the work done, then he merged them. Or maybe there's something I'm not grasping here

PRs are not mergeables if copier template is not updated, so doing the fixes in PR without updating copier template in the same time is un-mergeable.

I'll do a PR to #276 to make it mergeable.

@GSLabIt GSLabIt force-pushed the 14.0-account_fiscal_year_closing-migration_script branch from f910645 to dc80b4c Compare October 31, 2023 14:30
@francesco-ooops
Copy link
Contributor

@sergiocorato merge?

@sergiocorato
Copy link
Contributor

@sergiocorato merge?

I remember that 2 positive reviews are required to merge.

@francesco-ooops
Copy link
Contributor

@OCA/accounting-maintainers anyone can take a look at this migration script fix? thanks!

@sergiocorato
Copy link
Contributor

@OCA/accounting-maintainers anyone can take a look at this migration script fix? thanks!

Anyone can review this PR.

@francesco-ooops
Copy link
Contributor

Then we do have two positive reviews :)

image

@sergiocorato
Copy link
Contributor

Sorry, I'll change my glasses :)
/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 14.0-ocabot-merge-pr-275-by-sergiocorato-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit f857bbe into OCA:14.0 Oct 31, 2023
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@matteoopenf matteoopenf matteoopenf approved these changes

@sergiocorato sergiocorato sergiocorato approved these changes

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

Successfully merging this pull request may close these issues.
6 participants
@GSLabIt @francesco-ooops @matteoopenf @pedrobaeza @sergiocorato @OCA-git-bot