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.

Sign up for GitHub

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

Jump to bottom

Add callback to adjust() to make it observable by callers #197

Merged
merged 1 commit into from
Sep 20, 2023
Merged

Add callback to adjust() to make it observable by callers #197

merged 1 commit into from
Sep 20, 2023

Conversation

happz
Copy link
Collaborator

@happz happz commented Jul 28, 2023

Whenever a rule is inspected, the given callback is called. This gives adjust() callers a chance to observe which rules are applied to which fmf nodes.

@happz happz mentioned this pull request Jul 28, 2023
Merged
lukaszachy
lukaszachy reviewed Aug 1, 2023
View reviewed changes
Copy link
Collaborator

@lukaszachy lukaszachy left a comment

Choose a reason for hiding this comment

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

Looks good but would be nice to add a (unit) test.

@lukaszachy lukaszachy added this to the 1.3 milestone Aug 1, 2023
@lukaszachy lukaszachy added the enhancement New feature or request label Aug 1, 2023
psss
psss requested changes Aug 11, 2023
View reviewed changes
Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

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

Looks good, just two minor questions. And agree with @lukaszachy that test coverage would be nice.

fmf/base.py Outdated Show resolved Hide resolved
fmf/base.py Outdated Show resolved Hide resolved
@thrix thrix self-requested a review August 22, 2023 08:44
@thrix thrix requested a review from psss September 18, 2023 11:08
@thrix
Copy link
Collaborator

thrix commented Sep 18, 2023

@psss can you please rereview?

thrix
thrix approved these changes Sep 18, 2023
View reviewed changes
Copy link
Collaborator

@thrix thrix left a comment

Choose a reason for hiding this comment

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

LGTM

@thrix
Copy link
Collaborator

thrix commented Sep 18, 2023

@happz I assume centos8 and old python again in the way, maybe we want to drop support for cs8/rhel8 also for fmf @psss ?

@psss
Copy link
Collaborator

psss commented Sep 19, 2023

@happz I assume centos8 and old python again in the way, maybe we want to drop support for cs8/rhel8 also for fmf @psss ?

Yes, we've shortly touched that already and the agreement was to keep fmf and tmt aligned. Filed #212 to drop el8 support.

psss
psss approved these changes Sep 20, 2023
View reviewed changes
Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

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

Thanks for addressing comments, now looks good.

@happz @psss
Add callback to adjust() to make it observable by callers
ef65fbe 
Whenever a rule is inspected, the given callback is called. This gives
`adjust()` callers a chance to observe which rules are applied to which
fmf nodes.
@psss psss self-assigned this Sep 20, 2023
@psss psss merged commit ef65fbe into main Sep 20, 2023
12 checks passed
@psss psss deleted the adjust-rule-callback branch September 20, 2023 09:06
@lukaszachy lukaszachy mentioned this pull request Jan 8, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thrix thrix thrix approved these changes

@lukaszachy lukaszachy lukaszachy approved these changes

@psss psss psss approved these changes

Assignees

@psss psss

Labels
enhancement New feature or request
Projects
None yet
Milestone
1.3
Development

Successfully merging this pull request may close these issues.
4 participants
@happz @thrix @psss @lukaszachy