Skip to content

Conversation

LecrisUT
Copy link
Collaborator

@LecrisUT LecrisUT commented Sep 22, 2025

There is already the packaged go-based yq available. It does have some cli differences hence why there was quite a bit of debate on how to get python-yq in as well, but realistically it should not make a difference for us since the core jq compatibility is the same.


Pull Request Checklist

  • implement the feature
  • Revise usage of yq flags
    • -y is equivalent with -o yaml, however this is likely unnecessary (anymore?)
    • -r seems to be a no-op now
    • -e make sure the last output is not false or null: equivalent, just keep
    • -c compact output: no equivalent, just pipe it to jq -c
    • -S sort keys: change to "sort_keys(.)"

@LecrisUT LecrisUT added the ci | full test Pull request is ready for the full test execution label Sep 22, 2025
@github-project-automation github-project-automation bot moved this to backlog in planning Sep 22, 2025
@LecrisUT LecrisUT moved this from backlog to review in planning Sep 22, 2025
@LecrisUT LecrisUT moved this from review to implement in planning Sep 22, 2025
@LecrisUT LecrisUT self-assigned this Sep 22, 2025
@psss psss changed the title Switch to using the packaged yq Switch to using the packaged yq Sep 24, 2025
@psss
Copy link
Collaborator

psss commented Sep 24, 2025

Oh, yq has been already packaged? Nice! Finally, we'll be able to get rid of those ugly setup hacks! 🎉

happz pushed a commit that referenced this pull request Sep 25, 2025
This is a blocker for #4087.

Basically because we are referencing the tmt repo itself in the tests,
some of the tests are running steps from the `main` branch in this case

https://github.com/teemtee/tmt/blob/b19b58c55eabfe5bd30fe26a33b7452c9d64fb1e/plans/main.fmf#L13-L22
even if I delete those in #4087 because it is taking the `main` ref
instead of the current ref.

For now a quick workaround is to first merge this in `main` to avoid
installing `yq` if already present and then delete it later in #4087.

Signed-off-by: Cristian Le <git@lecris.dev>
@LecrisUT LecrisUT force-pushed the chore/yq branch 2 times, most recently from e869f4e to e77c95e Compare September 25, 2025 15:39
@LecrisUT LecrisUT assigned LecrisUT and unassigned LecrisUT Sep 25, 2025
@thrix thrix requested review from happz, thrix, psss and tcornell-bus October 1, 2025 09:25
@LecrisUT LecrisUT moved this from implement to review in planning Oct 2, 2025
@LecrisUT LecrisUT removed their assignment Oct 2, 2025
Copy link
Collaborator

@tcornell-bus tcornell-bus left a comment

Choose a reason for hiding this comment

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

I know very little about yq but based on the flag descriptions you gave the replacements and removals looks correct, just one verification question.

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 thrix moved this from review to merge in planning Oct 6, 2025
@psss psss added code | style Code style changes not affecting functionality test coverage Improvements or additions to test coverage of tmt itself labels Oct 7, 2025
@psss psss added this to the 1.59 milestone Oct 7, 2025
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
This is likely a no-op unlike the `jq -r`

Signed-off-by: Cristian Le <git@lecris.dev>
It should not be necessary

Signed-off-by: Cristian Le <git@lecris.dev>
yq does not have an equivalent, but instead we can just pipe it to `jq`

Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
@happz happz merged commit 6821416 into teemtee:main Oct 7, 2025
26 checks passed
@github-project-automation github-project-automation bot moved this from merge to done in planning Oct 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci | full test Pull request is ready for the full test execution code | style Code style changes not affecting functionality test coverage Improvements or additions to test coverage of tmt itself
Projects
Status: done
Development

Successfully merging this pull request may close these issues.

6 participants