Skip to content

Conversation

sivel
Copy link
Member

@sivel sivel commented May 29, 2025

This PR allows downstreams to patch REQUIRE_ANSIBLE_CORE_PIN in constants.py to a value of True to force EE definitions to contain a pin for the ansible-core specification.

This would help ensure that users building custom EEs would have to put thought into the version of ansible-core that they intend to be using, without unexpected upgrades in the building process pulling in a newer version they have not tested against.

No evaluation of the pin is done, this is a minor safety net.

@sivel sivel requested a review from a team as a code owner May 29, 2025 15:36
@sivel sivel force-pushed the require-core-pin branch from 7f144c9 to 91306b7 Compare May 29, 2025 15:47
@Shrews Shrews added needs_triage New item that needs to be triaged and removed needs_triage New item that needs to be triaged labels May 29, 2025
Copy link
Contributor

@Shrews Shrews left a comment

Choose a reason for hiding this comment

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

I'd much rather see this validation done in the UserDefinition.validate() method where everything else is validated.

@sivel
Copy link
Member Author

sivel commented May 29, 2025

I'd much rather see this validation done in the UserDefinition.validate() method where everything else is validated

Done.

Copy link
Contributor

@Shrews Shrews left a comment

Choose a reason for hiding this comment

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

Suggested an additional test for complete code coverage, and a spelling correction. Otherwise, LGTM.

Co-authored-by: David Shrewsbury <[email protected]>
@sivel
Copy link
Member Author

sivel commented May 29, 2025

I've accepted the review changes, looks like there is at minimum a linting issue which I'll resolve though. I just want to note that the resolution of these things doesn't mean this needs merged.

This is an idea, which we ultimately need to decide whether we want it or not.

@ansible ansible deleted a comment from sonarqubecloud bot May 29, 2025
Copy link
Contributor

@Shrews Shrews left a comment

Choose a reason for hiding this comment

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

LGTM, but not merging until we decide we actually want this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants