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

10.0 mig mgmtsystem hazard #228

Merged
merged 4 commits into from
Oct 17, 2018
Merged

Conversation

ngrandjean
Copy link

@ngrandjean ngrandjean commented Aug 1, 2018

MIG mgmtsystem_hazard to V10.0

#159

Copy link
Member

@etobella etobella left a comment

Choose a reason for hiding this comment

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

Just a few changes

To configure this module, you need to:

* go to Settings > Companies > Companies and select your company
* in the configuration tab, select the risk computation formulae
Copy link
Member

Choose a reason for hiding this comment

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

I cannot find the risk computation formulae

Copy link
Author

Choose a reason for hiding this comment

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

I didn't notice, but that's because the field has been moved to module hazard_risk since V8 actually. I'll remove that from here.

"""Hazards of the health and safety management system"""""

_name = "mgmtsystem.hazard"
_description = __doc__

This comment was marked as resolved.

'views/mgmtsystem_hazard_usage.xml',
'views/mgmtsystem_hazard_control_measure.xml',
'views/mgmtsystem_hazard_test.xml',
'data/mgmtsystem_hazard_hazard.xml',
Copy link
Member

Choose a reason for hiding this comment

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

IMHO, all the data are demo data.

'demo/mgmtsystem_hazard_probability.xml',
'demo/mgmtsystem_hazard_severity.xml',
'demo/mgmtsystem_hazard_type.xml',
'demo/mgmtsystem_hazard_usage.xml',

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

@max3903 max3903 added this to the 10.0 milestone Sep 4, 2018
@max3903
Copy link
Sponsor Member

max3903 commented Sep 4, 2018

@ngrandjean Can you squash the translation commits please?

@ngrandjean
Copy link
Author

@max3903 translation commits are squashed (at least I think I did it right but tell me if not) and code changed according to comments.

@max3903
Copy link
Sponsor Member

max3903 commented Sep 11, 2018

@ngrandjean The commits are duplicated. You need to rebase and drop half of them:

$ git rebase -i oca/10.0

screenshot from 2018-09-11 08-47-42

and when the rebase is done, you need to force-push your branch to Github (assuming origin is your fork):

$ git push -f origin 10.0-mig-mgmtsystem_hazard

@ngrandjean
Copy link
Author

@max3903 Ok, thanks a lot for your help. the '-f' was what I missed the first time.

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.

LGTM. No test.

@ngrandjean
Copy link
Author

@etobella I fixed the issues you mentionned, would you mind checking and telling me if there's still something wrong ? Thanks a lot !

Copy link
Member

@etobella etobella left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@pedrobaeza
Copy link
Member

pedrobaeza commented Oct 11, 2018

Can you please squash all initial commits from "Nadege Grandjean", as they don't add significant information?

@ngrandjean
Copy link
Author

@pedrobaeza Sorry but I don't really understand what you mean. Where do you want me to do that ?

@pedrobaeza
Copy link
Member

I ask to merge together the initial commits, as they are from the same author and don't add any significant information being split. You can use https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests guide a reference.

@ngrandjean
Copy link
Author

@pedrobaeza Ok, so you're talking about commits 13ecb70 to 0583072 made between 3/02/17 and 9/03/17 ?
(Sorry to bother you with that but I'd rather ask than risk to break commit history because of a misunderstanding)

@pedrobaeza
Copy link
Member

Well, I was thinking in all the commits from Nadege Grandjean:

seleccion_009

@ngrandjean
Copy link
Author

@pedrobaeza Ok thanks, it's done (correctly I hope)

@@ -1,6 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<openerp>
<data noupdate="1">
<odoo noupdate="">
Copy link
Member

Choose a reason for hiding this comment

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

Here it's missing the 1

[MIG] mgmtsystem_hazard: Migration to 10.0

[MIG] mgmtsystem_hazard: Migration to 10.0
@max3903 max3903 merged commit cdc5b39 into OCA:10.0 Oct 17, 2018
@pedrobaeza pedrobaeza mentioned this pull request Oct 17, 2018
31 tasks
flachica pushed a commit to flachica/management-system that referenced this pull request Mar 19, 2020
Signed-off-by pedrobaeza
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