Skip to content

[10.0] [MIG] Migration of server_environment_ir_config_parameter#640

Merged
guewen merged 6 commits into
OCA:10.0from
acsone:10.0-mig-server_environment_ir_config_parameter-dro
Jun 21, 2017
Merged

[10.0] [MIG] Migration of server_environment_ir_config_parameter#640
guewen merged 6 commits into
OCA:10.0from
acsone:10.0-mig-server_environment_ir_config_parameter-dro

Conversation

@rousseldenis

Copy link
Copy Markdown
Contributor

No description provided.

@rousseldenis rousseldenis changed the title [10.0] [MIG] [10.0] [MIG] Migration of server_environement_ir_config_parameter Dec 9, 2016
@rousseldenis rousseldenis changed the title [10.0] [MIG] Migration of server_environement_ir_config_parameter [10.0] [MIG] Migration of server_environment_ir_config_parameter Dec 9, 2016
@sbidoul

sbidoul commented Dec 28, 2016

Copy link
Copy Markdown
Member

@rousseldenis can you please forward port this too #676 ?

else:
newvals = vals
super(IrConfigParameter, rec).write(newvals)
return super(IrConfigParameter, rec).write(newvals)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rousseldenis this does not look right. The write must be inside the loop as we overwrite whatever is provided in vals with the values found in the config file, so each property may end up with a different value. So this commit changes the expected behaviour.

newvals = vals
return super(IrConfigParameter, rec).write(newvals)
super(IrConfigParameter, rec).write(newvals)
return True

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah yes, fixed in next commit, okay.

@lmignon

lmignon commented Mar 9, 2017

Copy link
Copy Markdown
Contributor

This one depends of #595

@lmignon

lmignon commented Mar 9, 2017

Copy link
Copy Markdown
Contributor

@sbidoul #676 forward ported

@lmignon lmignon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code review + test

@sbidoul
sbidoul force-pushed the 10.0-mig-server_environment_ir_config_parameter-dro branch from 7f675f8 to e04e420 Compare March 31, 2017 13:42
@sbidoul

sbidoul commented Mar 31, 2017

Copy link
Copy Markdown
Member

rebased

@sbidoul
sbidoul force-pushed the 10.0-mig-server_environment_ir_config_parameter-dro branch from 8af6cda to 35da9d6 Compare March 31, 2017 14:34
@sbidoul

sbidoul commented Mar 31, 2017

Copy link
Copy Markdown
Member

This one is now finished and migrated to new API.

@leemannd leemannd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code review and tested. Thank you! 👍

@atchuthan atchuthan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

code review 👍

@guewen guewen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice!

@guewen
guewen merged commit c2d572f into OCA:10.0 Jun 21, 2017
SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-tools (8.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.

7 participants