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

[WIP] business_requirement_deliverable_default #5

Merged

Conversation

victormmtorres
Copy link
Collaborator

@victormmtorres victormmtorres commented Jun 3, 2016


  

victor added 2 commits September 1, 2016 17:58
	modified:   business_requirement_deliverable_default/__openerp__.py
	modified:   business_requirement_deliverable_default/views/business_requirement_deliverable_default.xml
@victormmtorres victormmtorres force-pushed the 3_business_requirement_deliverable_default branch from e24f5f0 to f267d24 Compare September 1, 2016 10:01
	modified:   business_requirement_deliverable_default/tests/test_default_resources_from_product.py
@coveralls
Copy link

coveralls commented Sep 5, 2016

Coverage Status

Coverage increased (+0.8%) to 91.453% when pulling 3fb7b1a on victormartinelicocorp:3_business_requirement_deliverable_default into 8442e3f on OCA:8.0.

@elicoidal
Copy link
Contributor

👍
@dreispt @victormartinelicocorp can we merge this one?

@coveralls
Copy link

Coverage Status

Coverage increased (+1.08%) to 91.736% when pulling f487f43 on victormartinelicocorp:3_business_requirement_deliverable_default into 8442e3f on OCA:8.0.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+1.08%) to 91.736% when pulling f487f43 on victormartinelicocorp:3_business_requirement_deliverable_default into 8442e3f on OCA:8.0.

@victormmtorres
Copy link
Collaborator Author

I already merge your PR to improve README file. LGTM @elicoidal


* Possibility to create default resource lines for a given product. This allows
the user to have standard resource lines uploaded in the BR for deliverable
packages.
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

This is the relevant explanation.
remote the generic irrelevant text before this - it is already explained in the main module (DRY).

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

From the description, do we need this to be a separate module?
Any issue on having it in business_requirement_deliverable?

Copy link
Contributor

Choose a reason for hiding this comment

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

@dreispt We built this feature after and kept it separate but there is nothing against merging.
Original idea was to have some very basic features in the br main module without any extra and later build the stack on it, which is why the original br .contains only the few basic models.
I 'd rather keep it like that but am open for a good argument to merge.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree on the DRY approach.
I will add a link such as:
More information about business requirements <https://github.com/OCA/business-requirement/blob/8.0/business_requirement/README.rst>_

'name': 'Business Requirement Deliverable Default',
'category': 'Business Requirements Management',
'summary': 'Business Requirement Deliverable Default',
'version': '8.0.2.0.2',

Choose a reason for hiding this comment

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

Is not first version?

Copy link
Contributor

Choose a reason for hiding this comment

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

It has been in production for many month with several version in our repos.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.08%) to 91.736% when pulling 03dfaf1 on victormartinelicocorp:3_business_requirement_deliverable_default into 8442e3f on OCA:8.0.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+1.08%) to 91.736% when pulling 03dfaf1 on victormartinelicocorp:3_business_requirement_deliverable_default into 8442e3f on OCA:8.0.

update openerp.py and removed unused images
@coveralls
Copy link

coveralls commented Sep 12, 2016

Coverage Status

Coverage increased (+1.08%) to 91.736% when pulling 27aa22f on victormartinelicocorp:3_business_requirement_deliverable_default into 8442e3f on OCA:8.0.

Victor Martin added 2 commits September 12, 2016 19:35
Delete bus_req_module_diag.png
'business_requirement_deliverable',
],
'image': [
'static/description/icon.png',
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

overindented

@coveralls
Copy link

Coverage Status

Coverage increased (+1.08%) to 91.736% when pulling 01720b7 on victormartinelicocorp:3_business_requirement_deliverable_default into 8442e3f on OCA:8.0.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+1.08%) to 91.736% when pulling 01720b7 on victormartinelicocorp:3_business_requirement_deliverable_default into 8442e3f on OCA:8.0.

@coveralls
Copy link

coveralls commented Sep 12, 2016

Coverage Status

Coverage increased (+1.08%) to 91.736% when pulling 01720b7 on victormartinelicocorp:3_business_requirement_deliverable_default into 8442e3f on OCA:8.0.

@codecov-io
Copy link

Current coverage is 91.73% (diff: 100%)

Sunburst

No coverage report found for 8.0 at 8442e3f.

Powered by Codecov. Last update 8442e3f...03dfaf1

@dreispt
Copy link
Sponsor Member

dreispt commented Sep 12, 2016

Lint fixes needed.

@coveralls
Copy link

coveralls commented Sep 13, 2016

Coverage Status

Coverage increased (+1.08%) to 91.736% when pulling 9e82adc on victormartinelicocorp:3_business_requirement_deliverable_default into 8442e3f on OCA:8.0.

@victormmtorres
Copy link
Collaborator Author

@gurneyalex Could you help with https://runbot.odoo-community.org/runbot/build/3154235 ?
Status killed.

cc @elicoidal

@gurneyalex
Copy link
Member

@victormartinelicocorp I triggered a rebuild after restarting postgresql. ping me if this does not help.

@victormmtorres
Copy link
Collaborator Author

AFAIK an onchange is triggered from a form, and operates on a singleton, so a for loop shouldn't be needed.

Then after latest commits could be merged @elicoidal @dreispt @moylop260 ?

@elicoidal
Copy link
Contributor

👍

@dreispt
Copy link
Sponsor Member

dreispt commented Sep 15, 2016

👍

@dreispt dreispt merged commit bba00c8 into OCA:8.0 Sep 15, 2016
@elicoidal
Copy link
Contributor

Thanks!

elicoidal pushed a commit that referenced this pull request Oct 4, 2016
…le_cost

Improved the README.rst and icon for business_requirement_deliverable_cost.
elicoidal added a commit that referenced this pull request Oct 4, 2016
…nt_project

7 business requirement project
RawEvan pushed a commit to RawEvan/business-requirement that referenced this pull request Oct 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants