Skip to content

Conversation

@aThorp96
Copy link
Member

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 24, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@aThorp96 aThorp96 marked this pull request as ready for review March 25, 2025 20:09
@aThorp96
Copy link
Member Author

aThorp96 commented Apr 7, 2025

@vikram-raj Do you mind reviewing when you are able?

@aThorp96 aThorp96 requested a review from vikram-raj April 9, 2025 13:19
Copy link
Member

@vikram-raj vikram-raj left a comment

Choose a reason for hiding this comment

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

The list page uses spec.displayName, whereas the details page uses metadata.name as a user it looks broken because of difference in name. As this is a feature, can you confirm it with PM/UX?

@aThorp96 aThorp96 requested a review from vikram-raj May 6, 2025 19:20
Copy link
Member

@lokanandaprabhu lokanandaprabhu left a comment

Choose a reason for hiding this comment

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

Screenshot 2025-05-07 at 11 47 35 AM

In TaskRun list page, it still shows name instead of displayName. Is this fine?

@vikram-raj
Copy link
Member

@lokanandaprabhu IMO, we should show displayName here if it is available in TR. wdyt @aThorp96 ?

@aThorp96
Copy link
Member Author

aThorp96 commented May 9, 2025

@vikram-raj @lokanandaprabhu in that view the Task column's display name is not actually related to the Task object, but the name of the Task in the Pipeline.

This column could be changed to fallback on the Task's Display Name via the TaskRun's annotation tekton.dev/displayName, if the TaskRun is not associated with a Pipeline/PipelineRun. However I worry that could introduce further confusion since the column would then refer to two different fields, depending on the TaskRun.

@vikram-raj
Copy link
Member

@aThorp96 @lokanandaprabhu, in my opinion, let's go with Task metadata.name in TR list page. and lets see if we get any feedback on this from the users.

@vikram-raj vikram-raj changed the title feat: Implement support for Task spec.displayName SRVKP-5501: feat: Implement support for Task spec.displayName May 14, 2025
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented May 14, 2025

@aThorp96: This pull request references SRVKP-5501 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the feature to target the "4.20.0" version, but no target version was set.

Details

In response to this:

Implements https://issues.redhat.com/browse/SRVKP-5501

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 openshift-eng/jira-lifecycle-plugin repository.

Copy link
Member

@vikram-raj vikram-raj left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm label May 21, 2025
@vikram-raj
Copy link
Member

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 21, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aThorp96, vikram-raj

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@vikram-raj
Copy link
Member

/retest

1 similar comment
@vikram-raj
Copy link
Member

/retest

@vikram-raj
Copy link
Member

/retest

1 similar comment
@vikram-raj
Copy link
Member

/retest

@vikram-raj vikram-raj merged commit 1dcb403 into openshift-pipelines:main Jun 18, 2025
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants