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

[12.0][IMP] - Add contract tags #402

Merged
merged 1 commit into from
Dec 22, 2019
Merged

Conversation

sbejaoui
Copy link
Contributor

No description provided.

@sbejaoui sbejaoui added this to the 12.0 milestone Oct 24, 2019
@sbejaoui sbejaoui force-pushed the 12.0-contract_tags-sbj branch 2 times, most recently from 4639d2c to 6dd3a65 Compare October 24, 2019 15:02
@sbejaoui sbejaoui changed the title [WIP][IMP] - Add contract tags [12.0][IMP] - Add contract tags Oct 24, 2019
Copy link
Member

@flotho flotho left a comment

Choose a reason for hiding this comment

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

AFAIK, our famous @pedrobaeza will require a README compliant to the the OCA requirements

@@ -9,7 +9,7 @@

{
'name': 'Recurring - Contracts Management',
'version': '12.0.4.1.0',
'version': '12.0.4.1.1',
'category': 'Contract Management',
'license': 'AGPL-3',
'author': "OpenERP SA, "
Copy link
Member

@flotho flotho Oct 26, 2019

Choose a reason for hiding this comment

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

Are you sure of this OpenERP mention? Is it a legacy porting ?

Copy link
Member

Choose a reason for hiding this comment

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

Original code (although not too much of it now) was from account_analytic_analysis module.

Copy link
Member

@sbidoul sbidoul left a comment

Choose a reason for hiding this comment

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

LGTM. cc/ @gva-acsone

company_id = fields.Many2one(
'res.company',
string='Company',
default=lambda self: self.env.user.company_id,
Copy link
Member

Choose a reason for hiding this comment

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

I used to use _company_default_get but nowadays I'm not so sure

Copy link

@gva-acsone gva-acsone left a comment

Choose a reason for hiding this comment

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

+1 Functional test

@sbejaoui
Copy link
Contributor Author

@sbidoul

can you update your review please

Copy link
Member

@flotho flotho left a comment

Choose a reason for hiding this comment

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

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

@sbidoul
Copy link
Member

sbidoul commented Dec 22, 2019

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 12.0-ocabot-merge-pr-402-by-sbidoul-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Dec 22, 2019
Signed-off-by sbidoul
@OCA-git-bot OCA-git-bot merged commit 3b79dea into OCA:12.0 Dec 22, 2019
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at fa02bf1. Thanks a lot for contributing to OCA. ❤️

BT-etejeda pushed a commit to BT-etejeda/contract that referenced this pull request May 17, 2024
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants