-
Notifications
You must be signed in to change notification settings - Fork 31
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
Check dependencies #416
Check dependencies #416
Conversation
This checks whether a set of collections contain all of their own depoendencies. The intention is that this can be run on the ansible package to tell if there are any missing collections or conflicting collection versions in the package. Fixes ansible-community#268
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.
This looks great to me! Should probably be reviewed by someone with a little more context on dependency checking though.
if dependency_version not in SemVerSpec(dep_version_spec): | ||
errors.append(f'{collection_name} version_conflict:' | ||
f' {dep_name}-{str(dependency_version)} but needs' | ||
f' {dep_version_spec}') |
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.
This produces: community.okd version_conflict: kubernetes.core-2.3.0 but needs >=2.1.0,<2.3.0
which isn't bad but I could see it get lost in the noise during a build. Maybe we could prepend it with WARNING:
or something like that ? Similar thoughts in other places where we print them.
Edit: I mean like here: https://github.com/ansible-community/antsibull/pull/416/files#diff-2f5af13eca4b3f060e34a7c676b4de74a09f3453b5e081086380b9204681b15bR455
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.
Good idea, I implemented that in 88dd4b2.
The other place where they are printed is the validate-deps
subcommand, where (similar to other linting subcommands in antsibull-changelog and antsibull-docs) stdout output is only generated if something is broken. There's no need to prepend things, this will only make it harder to process the output in other tools.
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.
LGTM, thanks for pulling it together.
@abadger thanks for starting this and doing the actually hard work :) |
Yay! Thanks for getting it over the finish line felixfontein! As the
saying goes: the last mile is always the hardest!
…On Tue, Apr 26, 2022, 11:27 PM Felix Fontein ***@***.***> wrote:
@abadger <https://github.com/abadger> thanks for starting this and doing
the actually hard work :)
@briantist <https://github.com/briantist> @dmsimard
<https://github.com/dmsimard> thanks for reviewing this!
—
Reply to this email directly, view it on GitHub
<#416 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABTCWXEVENNEFZY3QVNFRDVHDF2XANCNFSM5UF3IEGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This continues #270 by adding a new antsibull-build subcommand
validate-deps
that runs the dependency check, and by also running the dependency check when the Ansible package is build.With this PR, rebuilding Ansible 5.6.0 outputs: