Skip to content

konflux: Make description of entries more readable #698

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

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

c3d
Copy link
Contributor

@c3d c3d commented Jun 16, 2025

Some of the entries were beginning with - default. This is confusing during code review 1, where you tend to read that as belonging to the previous item instead of the current one. Move default after description, which puts description right after - to avoid that problem.

@openshift-ci openshift-ci bot requested review from bpradipt and pmores June 16, 2025 14:04
@c3d c3d force-pushed the tekton-description-readability branch 3 times, most recently from 35f69ee to 3030f41 Compare June 16, 2025 14:13
@c3d
Copy link
Contributor Author

c3d commented Jun 17, 2025

/retest

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 17, 2025
Some of the entries were beginning with `- default`.
This is confusing during code review [1], where you tend to read that
as belonging to the previous item instead of the current one.
Move `default` after description, which puts description right after
`-` to avoid that problem.

[1]: openshift#678 (comment)

Signed-off-by: Christophe de Dinechin <[email protected]>
@c3d c3d force-pushed the tekton-description-readability branch from 3030f41 to 054595b Compare June 17, 2025 10:58
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 17, 2025
@c3d
Copy link
Contributor Author

c3d commented Jun 18, 2025

/retest

@littlejawa
Copy link
Contributor

I agree with the change, but just want to reiterate a warning I did in another place: these files are generated by Konflux, and updated automatically by it. It may or may not restore the alphabetical ordering in a future update.
I am in favor of doing the change and hoping for the best, but if it is restored later, it's not my fault ! :-p

Copy link
Member

@beraldoleal beraldoleal left a comment

Choose a reason for hiding this comment

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

This looks much better. I'm not sure if the same will hold true with Konflux, though. Unfortunately, this was previously overridden by some automation tools, and it was hard to maintain the correct order. Hopefully we won’t run into that again here.

Copy link
Member

@beraldoleal beraldoleal left a comment

Choose a reason for hiding this comment

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

Any reason for not starting with the name?

@beraldoleal
Copy link
Member

I agree with the change, but just want to reiterate a warning I did in another place: these files are generated by Konflux, and updated automatically by it. It may or may not restore the alphabetical ordering in a future update. I am in favor of doing the change and hoping for the best, but if it is restored later, it's not my fault ! :-p

Didn't say this answer. And this answer my comment above :)

@littlejawa
Copy link
Contributor

/retest

Copy link

openshift-ci bot commented Jun 18, 2025

@c3d: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/sandboxed-containers-operator-e2e 054595b link false /test sandboxed-containers-operator-e2e

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 27, 2025
@openshift-merge-robot
Copy link

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copy link
Contributor

Caution

There are some errors in your PipelineRun template.

PipelineRun Error
osc-registry no kind "ImageDigestMirrorSet" is registered for version "config.openshift.io/v1" in scheme "k8s.io/client-go/kubernetes/scheme/register.go:83"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants