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

[MIG] agreement: Migration to 12.0 #287

Merged
merged 13 commits into from
Mar 28, 2019
Merged

[MIG] agreement: Migration to 12.0 #287

merged 13 commits into from
Mar 28, 2019

Conversation

ygol
Copy link

@ygol ygol commented Mar 19, 2019

I started over. Replacing #259.

I include my previous [comment]#259 (comment)

I haven't added groups, permissions as they will be different when used by @alexis-via (part of account) than by @max3903 (specific module permissions).

I did add a top menu "agreement" though.

@murtuzasaleh murtuzasaleh mentioned this pull request Mar 20, 2019
Closed
@ygol
Copy link
Author

ygol commented Mar 20, 2019

@max3903 @bealdav @alexis-via @pedrobaeza
Would you check/confirm this PR correspond to what was discussed in #220 and thus merge it?
TIA

murtuzasaleh
murtuzasaleh suggested changes Mar 20, 2019
View reviewed changes
agreement/__init__.py Outdated Show resolved Hide resolved
agreement/__manifest__.py Outdated Show resolved Hide resolved
bealdav
bealdav suggested changes Mar 20, 2019
View reviewed changes
Copy link
Member

@bealdav bealdav left a comment

Choose a reason for hiding this comment

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

Thanks a lot for porting this.

Just small comments to improve override with other module.

agreement/models/agreement.py Show resolved Hide resolved
agreement/views/agreement.xml Outdated Show resolved Hide resolved
agreement/views/agreement.xml Show resolved Hide resolved
agreement/__init__.py Outdated Show resolved Hide resolved
agreement/__manifest__.py Outdated Show resolved Hide resolved
agreement/models/__init__.py Outdated Show resolved Hide resolved
agreement/models/agreement.py Outdated Show resolved Hide resolved
@murtuzasaleh
Copy link
Member

@ygol Please Add README.rst file.

@ygol
Copy link
Author

ygol commented Mar 20, 2019

Adding reference to #189
@max3903 @bealdav @alexis-via @pedrobaeza
Should I do anything about codecov tests?
Is this ready to be merged?
Thank you

@OCA-git-bot OCA-git-bot mentioned this pull request Mar 20, 2019
Closed
19 tasks
max3903
max3903 reviewed Mar 20, 2019
View reviewed changes
agreement/__manifest__.py Outdated Show resolved Hide resolved
max3903
max3903 approved these changes Mar 20, 2019
View reviewed changes
Copy link
Sponsor Member

@max3903 max3903 left a comment

Choose a reason for hiding this comment

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

Few changes in the manifest. Otherwise LGTM

@ygol
Copy link
Author

ygol commented Mar 21, 2019

@bealdav @murtuzasaleh
if everything looks fine for you, would you approve the changes? TIA

bealdav
bealdav approved these changes Mar 21, 2019
View reviewed changes
Copy link
Member

@bealdav bealdav left a comment

Choose a reason for hiding this comment

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

On my side it's OK, but I haven't seen you don't have done a normal migration from old commits and add your migration commits.

It's not a problem for me, I'm not the original author, then I approve.

Thanks a lot

@max3903 max3903 requested a review from alexis-via March 21, 2019 14:32
@max3903 max3903 added this to the 12.0 milestone Mar 21, 2019
max3903
max3903 reviewed Mar 21, 2019
View reviewed changes
agreement/__manifest__.py Outdated Show resolved Hide resolved
@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). 🤖

@ygol ygol mentioned this pull request Mar 25, 2019
Closed
@murtuzasaleh
Copy link
Member

@alexis-via Can you please merge this PR.

@murtuzasaleh murtuzasaleh mentioned this pull request Mar 27, 2019
Closed
@max3903 max3903 merged commit b2eeff6 into OCA:12.0 Mar 28, 2019
@ygol ygol deleted the 12.0-mig-agreement branch June 4, 2019 09:18
BT-etejeda pushed a commit to BT-etejeda/contract that referenced this pull request May 17, 2024
@bt-admin
Merge pull request OCA#287 from OCA/15.0
568435b 
Syncing from upstream OCA/contract (15.0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@max3903 max3903 max3903 approved these changes

@bealdav bealdav bealdav approved these changes

@murtuzasaleh murtuzasaleh murtuzasaleh approved these changes

@alexis-via alexis-via Awaiting requested review from alexis-via

Assignees

@ygol ygol

Labels
approved needs review ready to merge
Projects
None yet
Milestone
12.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ygol @murtuzasaleh @OCA-git-bot @max3903 @bealdav