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

Revert "Revert "Renaming to platform variables"" #934

Merged
merged 6 commits into from
Oct 16, 2024

Conversation

Tompage1994
Copy link
Collaborator

i.e. reopens Sean's #928

@Tompage1994 Tompage1994 requested a review from a team as a code owner October 15, 2024 14:09
Copy link
Collaborator Author

@Tompage1994 Tompage1994 left a comment

Choose a reason for hiding this comment

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

Generally happy. @sean-m-sullivan I have a few comments but nothing major

galaxy.yml Outdated Show resolved Hide resolved
roles/controller_ad_hoc_command_cancel/defaults/main.yml Outdated Show resolved Hide resolved
roles/controller_ad_hoc_command/meta/argument_specs.yml Outdated Show resolved Hide resolved
@sean-m-sullivan sean-m-sullivan force-pushed the revert-932-revert-928-devel branch 2 times, most recently from ddae653 to 4c78324 Compare October 16, 2024 04:37
collection_name: controller_configuration
collection_version: 2.10.0
collection_name: aap_configuration
collection_version: 3.0.0-devel
collection_repo: https://github.com/redhat-cop/aap_configuration/
collection_dependencies: awx.awx
Copy link
Collaborator

Choose a reason for hiding this comment

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

can this take a list?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not without changing it all I believe

Copy link
Collaborator

Choose a reason for hiding this comment

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

We will need to make that change to fix the workflows

Copy link
Collaborator Author

@Tompage1994 Tompage1994 Oct 16, 2024

Choose a reason for hiding this comment

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

Ahh, I see what you're referring to now... (not clear in the GH ui). It does take a space separated list

Suggested change
collection_dependencies: awx.awx
collection_dependencies: awx.awx ansible.eda ansible.hub

We would also need ansible.platform but that doesn't seem to be on galaxy so that may not be too helpful

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sean-m-sullivan I think you're involved with the ansible.platform collection... is it going to be on galaxy? (or am I missing it?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think so, we might need to change the workflow stuff but we can work on that later

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is not AFAIK, I was involved in the initial push of it, but have not been more recently, but it it is my understanding it will not be public.

CHANGELOG.rst Outdated Show resolved Hide resolved
@Tompage1994
Copy link
Collaborator Author

This is now good to go from my perspective

@Tompage1994
Copy link
Collaborator Author

We need to fix all the linting issues etc. before releasing though

Copy link
Collaborator

@djdanielsson djdanielsson left a comment

Choose a reason for hiding this comment

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

LGTM

@djdanielsson djdanielsson merged commit dc1d494 into devel Oct 16, 2024
6 of 13 checks passed
@djdanielsson djdanielsson deleted the revert-932-revert-928-devel branch October 16, 2024 18:40
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.

3 participants