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

16.0 mig agreement legal #8

Merged
merged 57 commits into from
Jun 8, 2023

Conversation

urvisha-serpentcs
Copy link

No description provided.

@sanchonuria
Copy link

Can you add the commits from #9 please?

@rousseldenis
Copy link
Contributor

/ocabot rebase

@OCA-git-bot
Copy link
Contributor

Congratulations, PR rebased to 16.0.

@rousseldenis
Copy link
Contributor

/ocabot migration agreement_legal

Copy link

@len-foss len-foss left a comment

Choose a reason for hiding this comment

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

LGTM

@len-foss
Copy link

Hello @ygol
Is it possible to move forward with this?

Besides the PR mentioned above, it would be nice to make the create method work in batch, split the create_agreement function into a method with only the logic and one that does the view stuff (for cleaner overrides, instead of doing self.env[res["res_model"]].browse(res["res_id"])...).

Also, comment all the settings that refer to modules that haven't been migrated, and other modules depend on this one (e.g. agreement_project).

@ygol
Copy link
Contributor

ygol commented May 30, 2023

Hello @ygol Is it possible to move forward with this?

Besides the PR mentioned above, it would be nice to make the create method work in batch, split the create_agreement function into a method with only the logic and one that does the view stuff (for cleaner overrides, instead of doing self.env[res["res_model"]].browse(res["res_id"])...).

Also, comment all the settings that refer to modules that haven't been migrated, and other modules depend on this one (e.g. agreement_project).

Hi @len-foss I believe this PR was opened by @urvisha-serpentcs . Right?

@len-foss
Copy link

Right, it seems the work is done so the only thing left is a maintainer review/merge the PR. Did I get something wrong?

@ygol
Copy link
Contributor

ygol commented May 31, 2023

Right, it seems the work is done so the only thing left is a maintainer review/merge the PR. Did I get something wrong?

oh no. sorry for the confusion. it just you made some recommendation/remarks in your review and I thought you asked me to address them.

@rousseldenis
Copy link
Contributor

/ocabot rebase

@OCA-git-bot
Copy link
Contributor

Congratulations, PR rebased to 16.0.

@rousseldenis
Copy link
Contributor

@urvisha-serpentcs Could you just put the version change in manifest.py in migration commit and not in pre-commit one ? Thanks

@rousseldenis
Copy link
Contributor

@JayVora-SerpentCS

@SkiBY
Copy link

SkiBY commented Jun 6, 2023

@urvisha-serpentcs there are some test cases covered in PR# you can use to increase score.

Copy link

@SkiBY SkiBY left a comment

Choose a reason for hiding this comment

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

LGTM
Code coverage to be increased

Copy link

@AntoniRomera AntoniRomera left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@sanchonuria sanchonuria left a comment

Choose a reason for hiding this comment

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

Can you add the commits from #9 please?

@len-foss
Copy link

len-foss commented Jun 8, 2023

Honestly the simpler way would be to merge the PR, and then do another PR to bring #9 and/or other fixes.

@rousseldenis do you have merge rights?

@rousseldenis
Copy link
Contributor

Honestly the simpler way would be to merge the PR, and then do another PR to bring #9 and/or other fixes.

@rousseldenis do you have merge rights?

Indeed we can do a further PR

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-8-by-rousseldenis-bump-nobump, awaiting test results.

@len-foss
Copy link

len-foss commented Jun 8, 2023

Thank you!

@OCA-git-bot OCA-git-bot merged commit 7cbcd00 into OCA:16.0 Jun 8, 2023
@OCA-git-bot
Copy link
Contributor

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