-
Notifications
You must be signed in to change notification settings - Fork 218
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
Fixes #583: Move to Ansible-core for CI tests #591
Conversation
rodvand
commented
Sep 15, 2021
- Moved from ansible-base to ansible-core
- Removed form_factor from interface and family from ip_address as they were marked deprecated/removed
- Add test for pynetbox/requests library in nb_lookup.py
- Substituted list comprehension to match sanity test
- Removed family from test data to match removal from module
- Ignore pylint test on nb_lookup due to bug False positive isinstance-second-argument-not-valid-type when testing typing.Iterable pylint-dev/pylint#3507 in pylint 2.6.0 which is used in ansible-test
- Remove deprecated form_factor and family - Bump pylint to 2.8.0 - Move from ansible-base to ansible-core
Closes #583 |
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'd suggest to remove the removals from this PR and wait with them for the next major release.
(Also you'd need a |
@rodvand, do you think you would be able to address @felixfontein comments. I will work with @FragmentedPacket and @jeremystretch to try to get a release cut and pushed to ansible galaxy before the next release of Ansible 4 this up coming Tuesday. I have said it a few times, but I want to stress how helpful you have been to right this ship. |
Working on it. |
Adjusted removed_in_verison.
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 to me.
@felixfontein @gundalow @rodvand any final comments or items before I merge?
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 (as far as I can judge :) )