-
Notifications
You must be signed in to change notification settings - Fork 65
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
remove webConsolePlugin from ToolchainClusterConfig #1068
Conversation
Pendo has been decommisioned, we don't need this plugin anymore Signed-off-by: Xavier Coulon <[email protected]>
Pendo has been decommisioned, we don't need this plugin anymore testing for host-operator: codeready-toolchain/host-operator#1068 same changes as in codeready-toolchain#1018 Signed-off-by: Xavier Coulon <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall but please do not update the go.mod dependency in this PR.
go.mod
Outdated
replace github.com/codeready-toolchain/api => github.com/xcoulon/api v0.0.0-20240725145329-0fc7541fe19e | ||
|
||
replace github.com/codeready-toolchain/toolchain-common => github.com/xcoulon/toolchain-common v0.0.0-20240725145759-056fcf98e945 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xcoulon as you know we don't update CRDs and dependencies to api/common in the same PR. The reason is that you can't merge api and CRD changes in this same time if you do it.
You would have to:
- merge API
- change your operator/CRD PR to update the dependency (remove
replace
) - wait for the operator/CRD PR to get green
- merge it
steps 2 and 3 can take time (especially if there are problems with e2e tests, like infra outage) and you risk blocking others.
It's much safer to update CRDs only, so you can merge the API and the operator PR in the same time and then create a new PR to update the dependency. So you don't block anyone who might need to make a different change to API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, got it! (I totally forgot about these constraints)
here are the PR which only deal with the removal in the CRD:
#1069
codeready-toolchain/member-operator#589
codeready-toolchain/toolchain-e2e#1021
Co-authored-by: Rajiv Senthilnathan <[email protected]>
Pendo has been decommisioned, we don't need this plugin anymore testing for host-operator: codeready-toolchain/host-operator#1068 same changes as in codeready-toolchain#1018 Signed-off-by: Xavier Coulon <[email protected]>
/test e2e |
Signed-off-by: Xavier Coulon <[email protected]>
Quality Gate passedIssues Measures |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexeykazakov, ranakan19, xcoulon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1068 +/- ##
==========================================
- Coverage 85.08% 85.06% -0.02%
==========================================
Files 55 55
Lines 5034 5034
==========================================
- Hits 4283 4282 -1
- Misses 580 581 +1
Partials 171 171 |
Pendo has been decommisioned, we don't need this plugin anymore testing for host-operator: codeready-toolchain/host-operator#1068 same changes as in #1018 --------- Signed-off-by: Xavier Coulon <[email protected]>
Pendo has been decommisioned, we don't need this plugin anymore
see also:
Signed-off-by: Xavier Coulon [email protected]