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: facts being gathered unnecessarily #374

Merged

Conversation

richm
Copy link
Contributor

@richm richm commented Jul 13, 2023

Cause: The comparison of the present facts with the required facts is
being done on unsorted lists.

Consequence: The comparison may fail if the only difference is the
order. Facts are gathered unnecessarily.

Fix: Use difference which works no matter what the order is. Ensure
that the fact gathering subsets used are the absolute minimum required.

Result: The role gathers only the facts it requires, and does
not unnecessarily gather facts.

Signed-off-by: Rich Megginson [email protected]

Cause: The comparison of the present facts with the required facts is
being done on unsorted lists.

Consequence: The comparison may fail if the only difference is the
order.  Facts are gathered unnecessarily.

Fix: Use `difference` which works no matter what the order is.  Ensure
that the fact gathering subsets used are the absolute minimum required.

Result: The role gathers only the facts it requires, and does
not unnecessarily gather facts.

Signed-off-by: Rich Megginson <[email protected]>
@richm
Copy link
Contributor Author

richm commented Jul 13, 2023

Fixes #373

@codecov
Copy link

codecov bot commented Jul 13, 2023

Codecov Report

Patch coverage has no change and project coverage change: -1.68 ⚠️

Comparison is base (07022f4) 13.70% compared to head (708d557) 12.03%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #374      +/-   ##
==========================================
- Coverage   13.70%   12.03%   -1.68%     
==========================================
  Files           8        8              
  Lines        1729     1729              
  Branches       71        0      -71     
==========================================
- Hits          237      208      -29     
- Misses       1492     1521      +29     
Flag Coverage Δ
sanity ?

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

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@richm
Copy link
Contributor Author

richm commented Jul 13, 2023

[citest]

@spetrosi spetrosi merged commit 00fd1e3 into linux-system-roles:main Jul 14, 2023
16 of 17 checks passed
@richm richm deleted the sort-required-facts-plus-subsets branch July 14, 2023 13:49
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