-
Notifications
You must be signed in to change notification settings - Fork 22
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
fix: allow to disable osdeps overload warnings #409
base: master
Are you sure you want to change the base?
Conversation
It's a common usage pattern, and `autoproj show` allows to see it.
It is a common pattern but from my experience, it's often unintended too. Did you consider adding an API to disable it on a per-package basis instead? |
Is it really ? My point I guess is that right now I have 5 warnings filling a page that have been there for years, making sure that the warning is thoroughly ignored anyways. I really don't think an API would cut it. I wanted to avoid adding something to the osdeps language. |
I see the point, but I think this should be a per-package option, so if an override is long lasting, the override itself could be flagged as "nowarn=true" or sililar. If it is just a short-term merge request, keeping the warning could be beneficial, to be reminded to remove the override after merging. If the MR cannot be merged upstream, the at least the warning can be removed. On the other hand, if someone has trouble with branches, the warnings can easily be activated. Also, a lot of my warnings are results from having multiple root package_sets defining the same dependency (and have to to be able to work standalone), here, the the per-package settign is useless, because it is a matter on the inclusion order (and not really an override). |
To be clear... this is a configuration flag and it is false by default. If you guys want the warnings, you keep them by default. |
TLDR: I have no objections against this. But: A lot of these warnings come from "overrides" like
These are often caused by independent package_sets which don't even know that something is defined elsewhere. What would be really nice, is if autoproj could determine an intersection between these sets and only error out if that intersection was empty (but that would also likely break some cases, where an osdep is actually meant as override). It looks like both gem and pip can kind of compute the "intersection" themselves:
or
both will install some version between 3.2. and 4.0 (if available), and give an error if there is no available version (e.g., with contradicting version specifications). |
It's a common usage pattern, and
autoproj show
allows to see it.