Bug 1804684: remove multiple badge from PipelineRuns #4365
Conversation
|
@sahil143: This pull request references Bugzilla bug 1804684, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
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/test-infra repository. |
openshift-ci-robot
added
bugzilla/valid-bug
|
/assign |
andrewballantyne
left a comment
andrewballantyne
left a comment
There was a problem hiding this comment.
I think we went the wrong direction on this PR, please let me know if you want to talk outside of github.
There was a problem hiding this comment.
This is not the best way to go with this solution... Please re-read the ticket @sahil143 - I posted a PR link to what Sam did to solve this issue... we should look to mirror that here.
There was a problem hiding this comment.
Checked on the viability of that solution, and it's not going to work for this use-case. We still cannot do this change, it breaks the usage of badges throughout the list pages.
There was a problem hiding this comment.
Turns out there is a spot in the code that saves this chunk of code from needing to to this logic... and that's in the resource-list.tsx file
|
/kind bug |
openshift-ci-robot
added
the
kind/bug
sahil143
changed the title
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
@sahil143: This pull request references Bugzilla bug 1804684, which is valid. DetailsIn response to this:
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/test-infra repository. |
sahil143
force-pushed
the
piprun-badge
branch
from
f784701 to
4007cf2
Compare
sahil143
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
andrewballantyne
left a comment
andrewballantyne
left a comment
There was a problem hiding this comment.
Walked through the code with @sahil143.
We looked at
- Pipelines Run page on the admin side
- Pipelines Run page on the dev side
- Serverless pages on the admin side
- Helm / Monitoring on the dev side have custom logic and thus won't need to worry about auto badges (if they need it)
I think this is a good change now that I've deep dived in the code with Sahil. It still feels a little weird, but I think this is the best solution. I'm going to test around a bit more to see if I missed something.
There was a problem hiding this comment.
Turns out there is a spot in the code that saves this chunk of code from needing to to this logic... and that's in the resource-list.tsx file
There was a problem hiding this comment.
I'd put this at the top of the import list, but otherwise this looks good.
There was a problem hiding this comment.
Moved import to the top
|
You're also failing lints @sahil143 |
send badge through props fixed lint errors
sahil143
force-pushed
the
piprun-badge
branch
from
4007cf2 to
fe6c018
Compare
should be fixed now |
andrewballantyne
left a comment
andrewballantyne
left a comment
There was a problem hiding this comment.
/lgtm
/assign @christianvogt
We'll need Christian to do the approval because of the public folder change.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andrewballantyne, christianvogt, sahil143 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
5 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
|
/retest Please review the full test history for this PR and help us cut down flakes. |
8 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@sahil143: All pull requests linked via external trackers have merged. Bugzilla bug 1804684 has been moved to the MODIFIED state. DetailsIn response to this:
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/test-infra repository. |
|
/cherrypick release-4.4 |
|
@sahil143: new pull request created: #4434 DetailsIn response to this:
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/test-infra repository. |
8 participants
Fixes:
https://issues.redhat.com/browse/ODC-3102
Analysis / Root cause:
Two dev preview badges on Pipeline Run Page
Solution Description:
removed extra badge from the Page
Browser conformance: