Fix removal of wanted files #174
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yajo
added a commit
that referenced
this pull request
Oct 11, 2018
This feature was introduced in #71, but time has proven it buggy and not much useful. This feature has a chicken-and-egg problem in the devel environment: code doesn't exist at build time, but it is expected to be able to find the `requirements.txt` files inside it. It confuses the developer and creates divergencies between devel and test/prod environments (where that problem doesn't exist). Also it's breaking #174, which is a more important and actually used feature. The best fix is to drop support for this. If you really want to have requirements from remotes, you can add this to your `dependencies/pip.txt` file: ``` # Assuming you are installing all from server-tools -r https://raw.githubusercontent.com/OCA/server-tools/11.0/requirements.txt ``` It's a better choice because: - It's basic official pip. - It's explicit. - It works fine across environments.
yajo
added a commit
that referenced
this pull request
Oct 11, 2018
This feature was introduced in #71, but time has proven it buggy and not much useful. This feature has a chicken-and-egg problem in the devel environment: code doesn't exist at build time, but it is expected to be able to find the `requirements.txt` files inside it. It confuses the developer and creates divergencies between devel and test/prod environments (where that problem doesn't exist). Also it's breaking #174, which is a more important and actually used feature. The best fix is to drop support for this. If you really want to have requirements from remotes, you can add this to your `dependencies/pip.txt` file: ``` # Assuming you are installing all from server-tools -r https://raw.githubusercontent.com/OCA/server-tools/11.0/requirements.txt ``` It's a better choice because: - It's basic official pip. - It's explicit. - It works fine across environments.
yajo
added a commit
to Tecnativa/doodba-scaffolding
that referenced
this pull request
Dec 4, 2018
Although it's not recommended nor officially supported, this simple change avoids syntax errors that can happen e.g. when compiling python code in Odoo <= 10.0 with a repo included in another, like with the use case explained in Tecnativa/doodba#174.
Previous implementation of `400-clean` build script was expecting a very specific directory structure of `repo/addons`, or otherwise it could be removing things that you actually wanted. Imagine for instance that 2 doodba projects share the same private addons, so you add them in `odoo/src/private` in the 1st one, and link them in the 2nd one with repos+addons combination, adding something like this to `addons.yaml`: ```yaml other-doodba/odoo/src/private: - custom_other_addon ``` Previous implementation of the clean step would be removing the `other-doodba` folder entierly, as it was not following the expected structure. This new implementation is smarter: 1. Preserve all Odoo source code, outside of its `addons` folder (which is handled as a normal addons repo, to be able to restrict them if wanted). 2. For other folders, check if it can be found inside an activated addons path. If not, remove it. 3. Repeat, recursively, bottom-up. It should be more flexible and less error-prone. A test that was testing the cleanup has been slightly modified to support this use case.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previous implementation of
400-cleanbuild script was expecting a very specific directory structure ofrepo/addons, or otherwise it could be removing things that you actually wanted.Imagine for instance that 2 doodba projects share the same private addons, so you add them in
odoo/src/privatein the 1st one, and link them in the 2nd one with repos+addons combination, adding something like this toaddons.yaml:Previous implementation of the clean step would be removing the
other-doodbafolder entierly, as it was not following the expected structure.This new implementation is smarter:
addonsfolder (which is handled as a normal addons repo, to be able to restrict them if wanted).It should be more flexible and less error-prone. A test that was testing the cleanup has been slightly modified to support this use case.