-
Notifications
You must be signed in to change notification settings - Fork 8
Improve convert #83
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
base: master
Are you sure you want to change the base?
Improve convert #83
Conversation
4ee9ea8 to
748e6b3
Compare
| for line in old_env_lines: | ||
| new_line = ( | ||
| line.replace( | ||
| "/odoo/src/addons", "/odoo/src/odoo/odoo/addons, /odoo/src/odoo/addons" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/odoo/src/odoo/odoo/addons I don't remember seeing it. It exists or is it a typo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's the path of the base addons. Not sure why I had issues without this. I'll double check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/odoo/src/addons + /odoo/src/odoo/addons should be enough IMO, the other one doesn't exist
| for line in old_env_lines: | ||
| new_line = ( | ||
| line.replace( | ||
| "/odoo/src/addons", "/odoo/src/odoo/odoo/addons, /odoo/src/odoo/addons" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The replace seems strange. I may be missing something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is 1 directory in the "old style" image, and I had to replace it with the 2 addons directories of Odoo (addons/ and odoo/addons/ ) in the new style image
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I only have one point on which I'm not sure.
748e6b3 to
6cf1f18
Compare
| shutil.move("odoo/Dockerfile", "Dockerfile.bak") | ||
| old_env_lines = [] | ||
| with open("Dockerfile.bak") as old_dockerfile: | ||
| found_env = False | ||
| for line in old_dockerfile: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now pathlib is mandatory for this repo.
| shutil.move("odoo/Dockerfile", "Dockerfile.bak") | |
| old_env_lines = [] | |
| with open("Dockerfile.bak") as old_dockerfile: | |
| found_env = False | |
| for line in old_dockerfile: | |
| old_dockerfile = Path("odoo/Dockerfile").rename("Dockerfile.bak") | |
| new_dockerfile = Path("Dockerfile") | |
| old_env_lines = [] | |
| found_env = False | |
| with old_dockerfile.open() as fobj: | |
| for line in fobj: |
| with open("Dockerfile") as new_dockerfile: | ||
| found_env = False | ||
| for line in new_dockerfile: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| with open("Dockerfile") as new_dockerfile: | |
| found_env = False | |
| for line in new_dockerfile: | |
| found_env = False | |
| with new_dockerfile.open() as fobj: | |
| for line in fobj: |
| with open("Dockerfile", "w") as new_dockerfile: | ||
| new_dockerfile.writelines(new_docker_file_lines) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| with open("Dockerfile", "w") as new_dockerfile: | |
| new_dockerfile.writelines(new_docker_file_lines) | |
| with new_dockerfile.open("w") as fobj: | |
| fobj.writelines(new_docker_file_lines) |
closes: #82