Cleanup tox.ini and GHA #439
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #439 +/- ##
=======================================
Coverage 35.67% 35.67%
=======================================
Files 143 143
Lines 11305 11305
Branches 2291 2291
=======================================
Hits 4033 4033
- Misses 7197 7272 +75
+ Partials 75 0 -75
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Build failed. ✔️ build-ansible-collection SUCCESS in 15m 35s |
|
recheck |
|
Build failed. ✔️ build-ansible-collection SUCCESS in 10m 20s |
|
This is weird. It looks like And: And now |
|
recheck |
|
Build failed. ✔️ build-ansible-collection SUCCESS in 14m 05s |
|
recheck |
|
Build failed. ✔️ build-ansible-collection SUCCESS in 10m 02s |
|
recheck |
|
Build succeeded. ✔️ build-ansible-collection SUCCESS in 10m 40s |
There was a problem hiding this comment.
This looks basically OK to me, but ansible-lint GHA fails for some integration tests. I'm not really sure what we should do:
- Fix things
- Make ansible-lint ignore those files
- Remove ansible-lint again (for now)
I tend to the last option so we can merge your other changes. And then work on ansible-lint in a second step. But that's only my opinion and I don't feel very strong on it.
What do you think?
|
@mariolenz : We do not run ansible-lint on the integration test targets. So I will add a rule to skip the tests folder. With respect to the sanity test failures seen on Zuul, we can ignore them. We will eventually remove the zuul jobs for this collection and move completely to GHA |
👍
Oh, I don't mind those. They didn't look related to your changes and, anyway, the last ansible/check on Zull succeeded. They just confused me and I wanted to document my findings ;-) |
|
@GomathiselviS ansible-lint failed again, but now with a lot of other issues. I'm afraid you'll have to have another look :-( |
|
Build failed. ✔️ build-ansible-collection SUCCESS in 10m 09s |
|
Build failed. ✔️ ansible-test-cloud-integration-vmware-rest SUCCESS in 18m 16s |
There was a problem hiding this comment.
I see you've dropped support for ansible-core < 2.14 now and set the collection version to 2.3.2. I'm not sure if we have guidelines for dropping ansible-core versions, but I know at least some people who consider this a breaking change. Which means a new major release, 3.0.0. Anyway, I think it's definitely not a bugfix. So you should increase at least the minor (2.4.0) and not only the patch version.
Personally, I would drop ansible-core < 2.14 in 3.0.0. That said, I feel a bit unsure about dropping support for ansible-core versions / doing a new major release "just like this". Can't we keep the required ansible (core) version at >=2.9.10 for now and get the GHA workflows running? And then prepare a new major release where we drop ansible-core < 2.14 and maybe do some other things (like completely re-generating everything with ansible.content_builder) properly? This feels a bit rash at the moment.
The upcoming release for this collection will be version 3.0.0. I will update the galaxy file and changelog accordingly. Please note that support for ansible-core 2.13 will be discontinued. Additional details can be found here. |
👍
Yes, I know ;-) It's a bit unfortunate since it's too late to get 3.0.0 into Ansible 9... anyway, let's do it! I'll prepare a PR to remove the 2.9 / 2.10 / 2.11 / 2.12 Zuul jobs! Please remember to remove 2.13 from the GHA sanity tests while you're at it. I think it's still mentioned. |
ansible/ansible-zuul-jobs#1832
Please also remove the ignore files 2.9 to 2.13 from tests/sanity/ because we don't need them anymore. |
|
I think |
|
Build succeeded. ✔️ ansible-test-cloud-integration-vmware-rest SUCCESS in 21m 11s |
|
Build succeeded. ✔️ ansible-test-cloud-integration-vmware-rest SUCCESS in 17m 36s |
| @@ -271,7 +270,7 @@ async def update_changed_flag(data, status, operation): | |||
|
|
|||
|
|
|||
| async def list_devices(session, url): | |||
| existing_entries = [] | |||
| pass | |||
There was a problem hiding this comment.
this seems unnecessary, I would suggest removing this. if we need to make changes to the generator to do that I'd be ok with merging this for now as it should be harmless and doing that under a separate PR.
There was a problem hiding this comment.
It might not be - I'm more familiar with the generated amazon collection and some of those module_utils lived in an early version of that generator, so calling it out just in case!
There was a problem hiding this comment.
@jillr It's already a very big PR with a lot of changes. I suggest to keep it like this and have a closer look at this later ;-)
|
recheck |
|
temp. closing |
|
Build succeeded. ✔️ ansible-test-cloud-integration-vmware-rest SUCCESS in 13m 04s |
|
@GomathiselviS @jillr Is there anything left to do? Or do you think we can merge when ansible/ansible-zuul-jobs#1832 is merged, too? |
|
@mariolenz I think this one is ready; I have some other zuul PRs to review as well so I will get that one later today |
|
Build failed (gate pipeline). For information on how to proceed, see https://ansible.softwarefactory-project.io/zuul/buildset/098546abbe8146bd815c4c2250a05b17 Warning: |
|
@GomathiselviS I think we might need to remove the Depends-On from this ticket, but I'm not certain. |
|
Build succeeded. ✔️ ansible-test-cloud-integration-vmware-rest SUCCESS in 22m 00s |
|
Build succeeded (gate pipeline). ✔️ ansible-test-cloud-integration-vmware-rest SUCCESS in 17m 37s |
|
Pull request merge failed: Resource not accessible by integration, You may need to manually rebase your PR and retry. |
I did, but still had to merge manually:
I think because the GHA stuff / files in .github, but I'm not 100% sure. |
That's right, thanks! |
SUMMARY
ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION