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

Use concrete step data class when injecting requires #3144

Merged
merged 1 commit into from
Aug 14, 2024
Merged

Use concrete step data class when injecting requires #3144

merged 1 commit into from
Aug 14, 2024

Conversation

happz
Copy link
Collaborator

@happz happz commented Aug 12, 2024

Besides raw data, Step.delegate() can accept data class instance too. Converting prepare step to use this method to provide better type safety.

Pull Request Checklist

  • implement the feature

@happz happz added code | style Code style changes not affecting functionality code | no functional change "No Functional Change" intended. Patch should not change tmt's behavior in any way. labels Aug 12, 2024
@happz happz added this to the 1.36 milestone Aug 12, 2024
@thrix thrix requested a review from martinhoyer August 13, 2024 11:42
thrix
thrix approved these changes Aug 13, 2024
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

@happz happz added the status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. label Aug 13, 2024
psss
psss approved these changes Aug 13, 2024
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.

Nice!

@psss psss added the ci | full test Pull request is ready for the full test execution label Aug 13, 2024
@psss psss force-pushed the prepare-requires-use-step-data branch 2 times, most recently from cea09fe to 36c55d7 Compare August 13, 2024 13:50
@happz
Use concrete step data class when injecting requires
1578129 
Besides raw data, `Step.delegate()` can accept data class instance too.
Converting `prepare` step to use this method to provide better type
safety.
@happz happz force-pushed the prepare-requires-use-step-data branch from 36c55d7 to 1578129 Compare August 13, 2024 17:21
@happz
Copy link
Collaborator Author

happz commented Aug 13, 2024

/packit build

@happz happz merged commit 156f0af into main Aug 14, 2024
19 checks passed
@happz happz deleted the prepare-requires-use-step-data branch August 14, 2024 09:04
The-Mule pushed a commit to The-Mule/tmt that referenced this pull request Oct 14, 2024
@happz @The-Mule
Use concrete step data class when injecting requires (teemtee#3144)
3ee6403 
Besides raw data, `Step.delegate()` can accept data class instance too.
Converting `prepare` step to use this method to provide better type
safety.
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

@psss psss psss approved these changes

@therazix therazix therazix approved these changes

@lukaszachy lukaszachy Awaiting requested review from lukaszachy lukaszachy is a code owner

@janhavlin janhavlin Awaiting requested review from janhavlin janhavlin is a code owner

@martinhoyer martinhoyer Awaiting requested review from martinhoyer

Assignees
No one assigned
Labels
ci | full test Pull request is ready for the full test execution code | no functional change "No Functional Change" intended. Patch should not change tmt's behavior in any way. code | style Code style changes not affecting functionality status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish.
Projects
None yet
Milestone
1.36
Development

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