-
Notifications
You must be signed in to change notification settings - Fork 18
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
Codependencies #19
Codependencies #19
Conversation
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.
Cool, thanks!
A few comments at this stage (I've not done a full review it, just skimmed through it).
Would you mind doing two PRs for the git and co-dependencies parts ? It will help (me at least) focusing the review.
.gitignore
Outdated
Session.vim | ||
Sessionx.vim | ||
*~ | ||
\#*\# |
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.
I like to keep my projects editor-agnostic. So I generally recommend putting this in your personal ~/.gitignore
via ~/.gitconfig
, such as:
[core]
editor = vim
excludesfile = ~/.gitignore
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.
I was wondering if that was the workaround you would suggest :-) Sure, will do it.
src/manifestoo/main.py
Outdated
include_selected: bool = typer.Option( | ||
True, | ||
"--include-selected", | ||
help="Print the selected addons along with their co-dependencies.", |
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.
Could you explain here what co-dependencies are ?
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.
Given a relation, it's dual (co-relation) is obtained by inverting the arrows. But I will put an explanation.
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.
A minor remark about the help text, otherwise looks very good, thanks!
Can you also add a news/7.feature
file containing a short changelog paragraph (rst format) ? I use towncrier to generate the changelog.
"(the set of addons that depends on the set of selected addons).", | ||
), | ||
) -> None: | ||
"""Print the co-dependencies of selected 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.
Le's put the explanation about what a co-dependency is here rather than in the --include-selected description ?
Closes #7 |
Any plans to merge it in a stable version yet? I am exactly in the raised case of:
that I would like to implement in our CI/CD |
@b-enoit-be Unfortunately I have a big project deadline right now, so I don't think I'll have a look at it before one month from now. However I don't think there's much to fix to merge, so feel free to add what's missing. |
Since I need to cut a release and this is almost finished, I merge and apply the last tweak in a followup. |
This PR adds a command to list co-dependencies of a set of addons, i.e. the set of addons that depend on them.
In most cases, to test a feature branch it is sufficient to test the modified modules and their co-dependencies.
Using #20, to put both together:
manifestoo --addons-path odoo/addons/ -g master list-codepends
No tests added yet.