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

fix(ansible-test-sanity): update workflow to run in every pr #652

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

Wabri
Copy link
Member

@Wabri Wabri commented Feb 12, 2024

Closes #648

@Wabri Wabri added the enhancement New feature or request label Feb 12, 2024
@Wabri Wabri requested a review from berndfinger February 12, 2024 16:58
@Wabri Wabri self-assigned this Feb 12, 2024
@Wabri Wabri force-pushed the feature/648/ansible-test-sanity branch from 58a6095 to 40558c7 Compare February 12, 2024 22:09
@Wabri Wabri changed the base branch from main to dev February 12, 2024 22:09
@ja9fuchs
Copy link
Contributor

This reverts 4fa8571.

We had decided to change it to a schedule in the past, because it has little value to see this check failing for the overall collection in any PR that only contains code changes in a role that is actually clean itself. That's what the role checks are for, to limit the lint checking to the parts that are affected by the PR, and where the result has a meaning for the particular PR.

@berndfinger
Copy link
Member

This reverts 4fa8571.

Only in part: The workflow for ansible-linting the collection will not be modified. The reason why I suggested to modify the trigger for the Ansible sanity testing is this issue, plus the low overhead for running this workflow. With this fix, errors outside of roles can be detected (and fixed) early and we will less likely overlook them.

@Wabri Wabri requested a review from ja9fuchs February 13, 2024 15:11
@Wabri
Copy link
Member Author

Wabri commented Feb 14, 2024

After the approval from @ja9fuchs + @berndfinger I'll merge the pr

Copy link
Contributor

@ja9fuchs ja9fuchs left a comment

Choose a reason for hiding this comment

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

Alright, let's see how it goes then. 👍

Copy link
Member

@berndfinger berndfinger left a comment

Choose a reason for hiding this comment

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

LGTM!

@berndfinger
Copy link
Member

@Wabri Andiamo! :-)

@Wabri
Copy link
Member Author

Wabri commented Feb 16, 2024

@berndfinger procedo!

@Wabri Wabri merged commit 5a790ac into sap-linuxlab:dev Feb 16, 2024
7 checks passed
@Wabri Wabri deleted the feature/648/ansible-test-sanity branch February 16, 2024 10:19
@berndfinger
Copy link
Member

FYI: https://github.com/sap-linuxlab/community.sap_install/actions/runs/7933809574/job/21663414670?pr=653 proves that it is working as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

collection/workflows: Run ansible-test sanity for each pull request
3 participants