Skip to content

[10.0][MIG] sale - #851

Merged
pedrobaeza merged 2 commits into
OCA:10.0from
rruebner:10.0-sale
Jun 29, 2017
Merged

[10.0][MIG] sale#851
pedrobaeza merged 2 commits into
OCA:10.0from
rruebner:10.0-sale

Conversation

@rruebner

Copy link
Copy Markdown

FYI: I also updated the analysis text file for the sale module here. The rest of the analysis text files were updated in #850.

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@cubells cubells 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.

Typo

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.

Typo: there are 4 extra spaces. :)

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.

Please check it if warning module was installed on v9 and rescue values from it.

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.

You can use openupgradelib map_values function for readibility and extra log.

@rruebner

Copy link
Copy Markdown
Author

@pedrobaeza I updated the migration parts according to your mentioned points. In addition I rechecked all parts again and added some more migration parts.

@pedrobaeza pedrobaeza 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.

Check if you need to rename the security groups

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.

s/their/there

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.

s/use/using

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.

s/than/then

@rruebner

Copy link
Copy Markdown
Author

@pedrobaeza I fixed the mentioned typos and rechecked security group changes. AFAICS there is no extra renaming needed (possible renaming is covered by update_module_names calls).
I also checked for noupdate=1 records and everything was fine.

@MiquelRForgeFlow MiquelRForgeFlow 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.

LGTM! 👍

@pedrobaeza

Copy link
Copy Markdown
Member

Thanks for the changes, Robert.

If you don't mind, I'm going to keep the PRs unmerged until we get a green branch for not adding more complexity to the pipe and possible side effects.

@MiquelRForgeFlow

Copy link
Copy Markdown
Contributor

Please, rebase the PR (and squash minor commits) ☺️

@MiquelRForgeFlow MiquelRForgeFlow 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.

A new line appeared https://github.com/OCA/OpenUpgrade/pull/917/files#diff-1c07737aeb217d5e147f8fa6d1204414R5, and it should be attended in an easy post-migration script.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If you don't have installed "sale_layout" module, columns renaming and the models renaming fails, maybe, you have to check if this module is installed to execute this two functions

@pedrobaeza

Copy link
Copy Markdown
Member

This fails due to TRAVIS_COMMIT, but I have checked it and fix it, so I merge.

@pedrobaeza
pedrobaeza merged commit 711a82a into OCA:10.0 Jun 29, 2017
@rruebner
rruebner deleted the 10.0-sale branch October 3, 2017 13:09
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.

6 participants