Skip to content
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

Bug 1994117: delete hardcodes and orphaned code #9870

Closed
wants to merge 1 commit into from

Conversation

maudem
Copy link
Contributor

@maudem maudem commented Aug 24, 2021

delete:

  • alert-manager.tsx
  • chargeback.tsx
  • cluster-service-class.tsx
  • instantiate-template.tsx
  • prometheus.jsx
  • service-instance.tsx
  • and associated code

@openshift-ci openshift-ci bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Aug 24, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 24, 2021

@maudem: This pull request references Bugzilla bug 1994117, which is invalid:

  • expected the bug to target the "4.9.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1994117: delete hardcodes and orphaned code

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 openshift-ci bot added the component/core Related to console core functionality label Aug 24, 2021
Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

Thanks @maudem. There are some additional things we need to check for and remove for all of these, including:

  1. Any orphaned CSS styles
  2. Any orphaned models in k8s-models.ts
  3. Any orphaned YAML templates in yaml-templates.ts
  4. Any orphaned feature flags (e.g., FLAGS.CHARGEBACK)
  5. Any orphaned table sorts or filters (table.tsx, table-filters.ts)
  6. Any orphaned tests for these components

All of service catalog should be removed, which includes some additional resources.

instantiate-template.tsx should NOT be removed.

@@ -1,387 +0,0 @@
import * as React from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

This file should NOT be removed. This is a feature we want to keep in console.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@spadgett this file was included in the hardcode list and in bugzilla. Was there another list I should have looked at for the files I needed to remove?

Copy link
Member

Choose a reason for hiding this comment

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

I think it was just incorrectly marked as one to remove.

@@ -1,19 +1,13 @@
import * as React from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

This entire file should be removed as well. Service catalog includes

Service Brokers
Service Classes
Service Instances
Service Bindings

Be careful about service bindings since there is another kind of service binding that is used in the dev console that should not be removed. Only the service catalog bindings should be removed.

We also need to remove the protractor tests for service catalog and any models as well as the custom table filters, YAML templates, and feature flags.

@@ -270,28 +269,6 @@ const AppContents: React.FC<{}> = () => {
}
/>

<LazyRoute
path="/catalog/instantiate-template"
Copy link
Member

Choose a reason for hiding this comment

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

instantiate-template should stay.

@@ -1,682 +0,0 @@
import * as React from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

@@ -17,7 +17,6 @@ import {
import { K8sResourceKind, referenceForModel } from '../module/k8s';
Copy link
Member

Choose a reason for hiding this comment

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

This entire file should be removed.

@@ -1,114 +0,0 @@
import * as React from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

Please remove any associated models.

@@ -1,164 +0,0 @@
import * as React from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

Please remove any associated models.

@@ -1,20 +1,7 @@
import * as React from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

This entire file should be removed.

@maudem
Copy link
Contributor Author

maudem commented Aug 25, 2021

/bugzilla refresh

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 27, 2021

@maudem: This pull request references Bugzilla bug 1994117, which is invalid:

  • expected the bug to target the "4.9.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1994117: delete hardcodes and orphaned code

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.

@maudem
Copy link
Contributor Author

maudem commented Sep 17, 2021

/bugzilla refresh

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 17, 2021

@maudem: This pull request references Bugzilla bug 1994117, which is invalid:

  • expected the bug to target the "4.10.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

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 openshift-ci bot added component/olm Related to OLM component/shared Related to console-shared kind/cypress Related to Cypress e2e integration testing kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated labels Sep 29, 2021
@maudem maudem force-pushed the bugfix-remove-obselete-code branch 2 times, most recently from 1ce8ccc to 0c75c20 Compare September 30, 2021 03:52
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 30, 2021
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 30, 2021
@maudem maudem force-pushed the bugfix-remove-obselete-code branch 2 times, most recently from 1ce8ccc to b22cb1c Compare September 30, 2021 14:03
@openshift-ci openshift-ci bot added component/sdk Related to console-plugin-sdk and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 4, 2021
@maudem maudem force-pushed the bugfix-remove-obselete-code branch from 7054346 to 2eaeac0 Compare October 8, 2021 14:54
@openshift-ci openshift-ci bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 8, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 8, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: maudem
To complete the pull request process, please assign vojtechszocs after the PR has been reviewed.
You can assign the PR to them by writing /assign @vojtechszocs in a comment when ready.

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

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

@maudem
Copy link
Contributor Author

maudem commented Oct 21, 2021

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 21, 2021

@maudem: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test analyze
  • /test backend
  • /test ceph-storage-plugin
  • /test e2e-gcp-console
  • /test frontend
  • /test images
  • /test kubevirt-plugin

Use /test all to run the following jobs that were automatically triggered:

  • pull-ci-openshift-console-master-analyze
  • pull-ci-openshift-console-master-backend
  • pull-ci-openshift-console-master-e2e-gcp-console
  • pull-ci-openshift-console-master-frontend
  • pull-ci-openshift-console-master-images

In response to this:

/retest required

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.

@maudem
Copy link
Contributor Author

maudem commented Oct 21, 2021

/retest

1 similar comment
@maudem
Copy link
Contributor Author

maudem commented Nov 1, 2021

/retest

@maudem
Copy link
Contributor Author

maudem commented Nov 1, 2021

/retest-required

2 similar comments
@maudem
Copy link
Contributor Author

maudem commented Nov 2, 2021

/retest-required

@maudem
Copy link
Contributor Author

maudem commented Nov 8, 2021

/retest-required

@maudem
Copy link
Contributor Author

maudem commented Nov 11, 2021

/hold e2e test pass blocked by https://bugzilla.redhat.com/2022452

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 11, 2021
@maudem
Copy link
Contributor Author

maudem commented Nov 15, 2021

/retest-required

delete references to cluster service broker in yaml-template

delete alert-manager and associated code

delete chargeback.tsx and associated code

delete cluster-service-class.tsx, instantiate-template.tsx,  and associated code

delete prometheus.jsx and associated code

delete service-instance.tsx and associated code

Bring back instantiate-template.tsx

Revert "delete files for Service Brokers"

This reverts commit 9f40bd78cb0fdaac10e8d6aadc188818c6633d23.

bring back instantiate-template

delete files for Service Brokers

delete Service Brokers in yaml-templates

Revert "delete Service Brokers in yaml-templates"

This reverts commit 36bcbb7b31b6947aa4a5f84ce62cebb0286c89e9.

remove broker management file

delete references to cluster service broker model in resource pages and lazy routes for provisioned services

find last remaining links and references to cluster service brokers, provisioned services, and broker mgt

delete unused models

delete custom styling and lazyroutes for alert manager

delete alert manager tests and such

delete more code relater to cluster service classes

delete custom service instance styles

update translations

Update public.json
@maudem
Copy link
Contributor Author

maudem commented Nov 24, 2021

/retest-required

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 24, 2021

@maudem: 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/e2e-gcp-console 81a74c1 link true /test e2e-gcp-console

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/test-infra repository. I understand the commands that are listed here.

@openshift-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 22, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 22, 2022

@maudem: 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/test-infra repository.

@openshift-bot
Copy link
Contributor

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 24, 2022
@openshift-bot
Copy link
Contributor

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci openshift-ci bot closed this Apr 24, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 24, 2022

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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
Copy link
Contributor

openshift-ci bot commented Apr 24, 2022

@maudem: This pull request references Bugzilla bug 1994117. The bug has been updated to no longer refer to the pull request using the external bug tracker. All external bug links have been closed. The bug has been moved to the NEW state.

In response to this:

Bug 1994117: delete hardcodes and orphaned code

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. component/core Related to console core functionality component/olm Related to OLM component/sdk Related to console-plugin-sdk component/shared Related to console-shared do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/cypress Related to Cypress e2e integration testing kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. 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.

None yet

3 participants