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

Rename the result phase to subresult #3106

Merged
merged 1 commit into from
Aug 13, 2024
Merged

Rename the result phase to subresult #3106

merged 1 commit into from
Aug 13, 2024

Conversation

seberm
Copy link
Collaborator

@seberm seberm commented Jul 23, 2024

In today's review meeting, we discussed that it might be a good idea to use a different name for "phase" results, such as "subresult".

The term "phase" was initially used in reference to beakerlib and the rlPhase{Start,End} functions. However, the name should be probably more general and not dependent on the used framework. Additionally, the phase keyword is used internally for different purposes.

This relates to:

CC: @happz, @thrix, @psss

Pull Request Checklist

  • implement the feature
  • update the specification
  • modify the json schema
  • mention the version

@seberm seberm marked this pull request as ready for review July 23, 2024 14:14
@seberm seberm self-assigned this Jul 23, 2024
@seberm seberm added area | results Related to how tmt stores and shares results code | schema Schema used for validating config files labels Jul 23, 2024
@seberm seberm force-pushed the feature/phase-to-subresult branch from bf6e97e to 1179884 Compare July 24, 2024 08:31
@thrix thrix added this to the 1.36 milestone Jul 31, 2024
@seberm seberm force-pushed the feature/phase-to-subresult branch 2 times, most recently from e6f7ddd to b8eb793 Compare August 5, 2024 11:39
@seberm
Copy link
Collaborator Author

seberm commented Aug 5, 2024

Do you have any additional ideas or suggestions? Are there any other places where the rename needs to be applied?

tmt/steps/prepare/shell.py Outdated Show resolved Hide resolved
@seberm seberm force-pushed the feature/phase-to-subresult branch 3 times, most recently from e58a303 to 896dc2e Compare August 7, 2024 10:55
@happz happz added the ci | full test Pull request is ready for the full test execution label Aug 7, 2024
@seberm seberm force-pushed the feature/phase-to-subresult branch from 896dc2e to a286d2a Compare August 8, 2024 06:56
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.

Yeah, makes sense. Looks good, just one typo.

tmt/schemas/results.yaml Outdated Show resolved Hide resolved
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!

@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
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, as there is nobody using this, so it should not break Testing Farm or any other user.

@thrix thrix added breakage Change will change current behaviour and removed breakage Change will change current behaviour labels Aug 13, 2024
@psss psss merged commit efa026f into main Aug 13, 2024
20 checks passed
@psss psss deleted the feature/phase-to-subresult branch August 13, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area | results Related to how tmt stores and shares results ci | full test Pull request is ready for the full test execution code | schema Schema used for validating config files status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants