Skip to content
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

Prep 3.0.0 using vSphere API version 7.0.3 #451

Conversation

alinabuzachis
Copy link
Collaborator

@alinabuzachis alinabuzachis commented Nov 22, 2023

SUMMARY

Prep 3.0.0 using vSphere API version 7.0.3

Release date is Monday November 27.

ISSUE TYPE
- New Module Pull Request
COMPONENT NAME

several

Copy link

@alinabuzachis
Copy link
Collaborator Author

alinabuzachis commented Nov 22, 2023

@mariolenz @jillr Integration tests seem passing in the CI with the new new API version. I used this one #449

@mariolenz
Copy link
Collaborator

mariolenz commented Nov 22, 2023

Cool!

There are a lot of linter errors, though. I think we can fix those Imports are incorrectly sorted and/or formatted by changing the order here:

https://github.com/ansible-community/ansible.content_builder/blob/1a8071e914aa1f25a84670b320c2295736a00818/roles/module_openapi_cloud/templates/module_directory/vmware_rest/header.j2#L30-L41

I'll create a PR for that and then maybe we can try again.

@mariolenz
Copy link
Collaborator

mariolenz commented Nov 22, 2023

@alinabuzachis Did the three new mappings I've suggested fix generating the modules? If yes, I would add them to my PR.

@mariolenz
Copy link
Collaborator

@alinabuzachis @jillr ansible-community/ansible.content_builder#74

Once this one and ansible-community/ansible.content_builder#73 are merged, I think we should give it another try 😃

@alinabuzachis
Copy link
Collaborator Author

alinabuzachis commented Nov 22, 2023

@alinabuzachis Did the three new mappings I've suggested fix generating the modules? If yes, I would add them to my PR.

Yes, they fixed. Can you please add them here ansible-community/ansible.content_builder#73?

@alinabuzachis
Copy link
Collaborator Author

Signed-off-by: Alina Buzachis <abuzachis@redhat.com>
Signed-off-by: Alina Buzachis <abuzachis@redhat.com>
Signed-off-by: Alina Buzachis <abuzachis@redhat.com>
Signed-off-by: Alina Buzachis <abuzachis@redhat.com>
Copy link

Signed-off-by: Alina Buzachis <abuzachis@redhat.com>
Signed-off-by: Alina Buzachis <abuzachis@redhat.com>
Signed-off-by: Alina Buzachis <abuzachis@redhat.com>
@alinabuzachis alinabuzachis changed the title [TEST] Prep 3.0.0 using vSphere API version 7.0.3 Prep 3.0.0 using vSphere API version 7.0.3 Nov 23, 2023
Copy link

Copy link
Collaborator

@mariolenz mariolenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alinabuzachis There are still a lot of failing CI jobs, and I think we should fix them before merging. I'm sure ansible-community/ansible.content_builder#73 and ansible-community/ansible.content_builder#74 would, but we need someone to merge them. I've tested in #452 and this looked OK to me. Of course, you can patch my changes into your local content_builder but having them upstream would be better.

One other thing: I've removed all module documentation from the community.vmware repository. It just felt like a waste of resources to generate them and drag them around. The stuff is available in the Ansible community documentation, anyway... and your module documentation is, too. Maybe you should consider this also. Just an idea ;-)

Since this doesn't concern the published collection, I don't think this would need a new major release and you could do it anytime. But preparing one, anyway, seems like a good moment to do it. That is, if you want to do it... as I've said: Just an idea.

Signed-off-by: Alina Buzachis <abuzachis@redhat.com>
Copy link

codecov bot commented Nov 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1939c7d) 35.67% compared to head (32e08ed) 34.15%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #451      +/-   ##
==========================================
- Coverage   35.67%   34.15%   -1.52%     
==========================================
  Files         143      143              
  Lines       11305    11055     -250     
  Branches     2291     2294       +3     
==========================================
- Hits         4033     3776     -257     
- Misses       7272     7279       +7     
Flag Coverage Δ
sanity 34.15% <ø> (-1.52%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alinabuzachis
Copy link
Collaborator Author

@alinabuzachis There are still a lot of failing CI jobs, and I think we should fix them before merging. I'm sure ansible-community/ansible.content_builder#73 and ansible-community/ansible.content_builder#74 would, but we need someone to merge them. I've tested in #452 and this looked OK to me. Of course, you can patch my changes into your local content_builder but having them upstream would be better.

One other thing: I've removed all module documentation from the community.vmware repository. It just felt like a waste of resources to generate them and drag them around. The stuff is available in the Ansible community documentation, anyway... and your module documentation is, too. Maybe you should consider this also. Just an idea ;-)

Since this doesn't concern the published collection, I don't think this would need a new major release and you could do it anytime. But preparing one, anyway, seems like a good moment to do it. That is, if you want to do it... as I've said: Just an idea.

Fixed linters and sanity errors! I tested too and those patches look good to me! Thanks!

We have the same way of handing docs in other collections too (see amazon.aws https://docs.ansible.com/ansible/latest/collections/amazon/aws/index.html). I'll try to add the Github workflow in another PR and eventually merged it after the 3.0.0 release. We can always do minor releases.

Copy link

Copy link
Collaborator

@mariolenz mariolenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

content_builder has created new sanity ignores for 2.9 to 2.13. We don't need them anymore, but I don't think we have to remove them now. We can do this later, no problem.

Maybe someone else should also have a look at this. Nearly 300 changed files is quite a lot and it's easy to overlook something.

Copy link

Build succeeded (gate pipeline).
https://ansible.softwarefactory-project.io/zuul/buildset/6e617dfed896445caef4425f57119853

✔️ ansible-test-cloud-integration-vmware-rest SUCCESS in 15m 56s
✔️ build-ansible-collection SUCCESS in 10m 12s
✔️ tox-cloud-refresh-examples-vmware SUCCESS in 11m 01s
✔️ ansible-galaxy-importer SUCCESS in 3m 36s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit be7e57d into ansible-collections:main Nov 27, 2023
10 of 11 checks passed
@mariolenz
Copy link
Collaborator

🥳

@alinabuzachis alinabuzachis mentioned this pull request Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants