Move topology from dev-console to its own monorepo #7254

jeff-phillips-18 · 2020-11-18T12:40:16Z

Fixes:
https://issues.redhat.com/browse/ODC-5037

Solution Description:
Created a new topology package, moved code from dev-console package there.

Non-functional change.

Browser conformance:

Chrome
Firefox
Safari
Edge

/kind cleanup

jeff-phillips-18 · 2020-11-18T15:04:51Z

/test analyze

jeff-phillips-18 · 2020-11-18T19:07:29Z

/retest

andrewballantyne · 2020-11-20T13:44:15Z

frontend/packages/dev-console/src/components/NamespacedPage.tsx

@@ -1,7 +1,7 @@
 import * as React from 'react';
 import * as cx from 'classnames';
 import { NamespaceBar } from '@console/internal/components/namespace';
-import ApplicationSelector from './dropdown/ApplicationSelector';
+import { NamespaceBarApplicationSelector } from '@console/topology/src';


Nit:

Suggested change

import { NamespaceBarApplicationSelector } from '@console/topology/src';

import { NamespaceBarApplicationSelector } from '@console/topology';

andrewballantyne · 2020-11-20T13:45:35Z

frontend/packages/dev-console/src/components/NamespacedPage.tsx

-      {!hideApplications && <ApplicationSelector disabled={disabled} />}
+      {!hideApplications && <NamespaceBarApplicationSelector disabled={disabled} />}


🤔 Is this PR not just a refactor... why the change in component usage?

There were two different ApplicationSelector components, one in shared one in dev-console. Both are now in topology (since the use the topology data retrieval) and this one was renamed.

Gotcha. Thanks for the clarification!

andrewballantyne · 2020-11-20T13:46:41Z

...tend/packages/dev-console/src/components/edit-application/__tests__/edit-application-data.ts

@@ -1,6 +1,6 @@
 import { K8sResourceKind } from '@console/internal/module/k8s';
 import { ServiceModel } from '@console/knative-plugin';
-import { UNASSIGNED_KEY } from '../../../const';
+import { UNASSIGNED_KEY } from '@console/topology/src/const';


Nit:

Suggested change

import { UNASSIGNED_KEY } from '@console/topology/src/const';

import { UNASSIGNED_KEY } from '@console/topology';

andrewballantyne · 2020-11-20T13:46:53Z

frontend/packages/dev-console/src/components/edit-application/edit-application-utils.ts

@@ -10,7 +10,7 @@ import { DeploymentConfigModel, DeploymentModel } from '@console/internal/models
 import { hasIcon } from '@console/internal/components/catalog/catalog-item-icon';
 import { ServiceModel } from '@console/knative-plugin';
 import { Pipeline } from '@console/pipelines-plugin/src/utils/pipeline-augment';
-import { UNASSIGNED_KEY } from '../../const';
+import { UNASSIGNED_KEY } from '@console/topology/src/const';


Nit:

Suggested change

import { UNASSIGNED_KEY } from '@console/topology/src/const';

import { UNASSIGNED_KEY } from '@console/topology';

andrewballantyne · 2020-11-20T13:49:13Z

frontend/packages/dev-console/src/components/import/CheIcon.tsx

@@ -16,5 +16,3 @@ const CheIcon: React.FC<React.HTMLProps<SVGElement>> = ({ style }): React.ReactE
    </svg>
  );
 };
-
-export default CheIcon;


Sole component file should have a default export... why the change?

Recommend you undo.

andrewballantyne · 2020-11-20T14:29:11Z

frontend/packages/topology/src/components/svg/SvgBoxedText.tsx

@@ -111,5 +111,3 @@ const SvgBoxedText: React.FC<SvgBoxedTextProps> = ({
    </g>
  );
 };
-
-export default SvgBoxedText;


andrewballantyne · 2020-11-20T14:29:34Z

frontend/packages/topology/src/const.ts

@@ -1,4 +1,6 @@
-import { STORAGE_PREFIX } from '@console/shared';
+import { STORAGE_PREFIX } from '@console/shared/src/constants/common';


Suggested change

import { STORAGE_PREFIX } from '@console/shared/src/constants/common';

import { STORAGE_PREFIX } from '@console/shared';

andrewballantyne · 2020-11-20T14:29:47Z

frontend/packages/topology/src/data-transforms/DataModelProvider.tsx

@@ -29,5 +29,3 @@ const DataModelProvider: React.FC<DataModelProviderProps> = ({ namespace, childr
    </ModelContext.Provider>
  );
 };
-
-export default DataModelProvider;


Default export

andrewballantyne · 2020-11-20T14:30:37Z

frontend/packages/topology/src/data-transforms/ModelContext.ts

-const ModelContext = createContext<ExtensibleModel>(null);
-
-export default ModelContext;
+export const ModelContext = createContext<ExtensibleModel>(null);


I would think this should still be a default export but it's odd it was put into a variable first.

default export createContext<ExtensibleModel>(null);

There are multiple exports.

😕 Sure, but in these 3 lines, it didn't need to create a variable to do a default export -- but I think you handled this in your last change, so it's moot now.

andrewballantyne · 2020-11-20T14:34:53Z

frontend/public/components/overview/OverviewListPage.tsx

-import { TopologyViewType } from '@console/dev-console/src/components/topology/topology-types';
+import { STORAGE_PREFIX } from '@console/shared';
+import { TopologyPage } from '@console/topology';
+import { TopologyViewType } from '@console/topology/src/topology-types';


Suggested change

import { TopologyViewType } from '@console/topology/src/topology-types';

import { TopologyViewType } from '@console/topology';

andrewballantyne

frontend/packages/topology/src/index.ts is an empty file. Perhaps delete?

andrewballantyne · 2020-11-23T17:19:01Z

frontend/packages/knative-plugin/src/components/add/channels/sections/DefaultChannelSection.tsx

 import { InputField } from '@console/shared';
 import { useTranslation } from 'react-i18next';
+import ApplicationSelector from '@console/topology/src/components/dropdowns/ApplicationSelector';
+import FormSection from '@console/dev-console/src/components/import/section/FormSection';


Nit: move these back up to where the other calls were.

andrewballantyne · 2020-11-23T17:26:45Z

frontend/packages/topology/src/__tests__/TopologySideBar.spec.tsx


 describe('TopologySideBar:', () => {
-  const props: TopologySideBarProps = {


hmmm... this could be to our loss if the type changes shape.

we'd get compile errors or test failures. A lot of tests don't use prop types.

Ah okay, no worries.

andrewballantyne · 2020-11-23T17:27:27Z

frontend/packages/topology/src/components/dropdowns/ApplicationDropdown.tsx

+import { useExtensions } from '@console/plugin-sdk/src';
+import { isTopologyDataModelFactory, TopologyDataModelFactory } from '../../extensions';
+import { WatchK8sResources } from '@console/internal/components/utils/k8s-watch-hook';
+import { getBaseWatchedResources } from '../../data-transforms/transform-utils';


andrewballantyne · 2020-11-23T18:12:00Z

/lgtm

Tested various things on Topology (adding items and seeing if sidebars loaded and pods spooled up and all the normal touch points I could think of) looks like the refactor didn't cause any issues as far as I can tell.

Nits are nice to haves, but I don't think they change the merge-ability of this PR.

christianvogt · 2020-11-23T18:32:20Z

/approve

andrewballantyne

/lgtm

openshift-ci-robot · 2020-11-23T18:48:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewballantyne, christianvogt, jeff-phillips-18

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:

~~frontend/OWNERS~~ [christianvogt]

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

jeff-phillips-18 requested a review from andrewballantyne November 18, 2020 12:40

openshift-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. component/core Related to console core functionality labels Nov 18, 2020

openshift-ci-robot requested review from christianvogt and glekner November 18, 2020 12:40

openshift-ci-robot added component/dev-console Related to dev-console component/knative Related to knative-plugin component/kubevirt Related to kubevirt-plugin component/shared Related to console-shared labels Nov 18, 2020

jeff-phillips-18 force-pushed the topology-package branch from 6198600 to 25ea9e5 Compare November 18, 2020 14:31

jeff-phillips-18 force-pushed the topology-package branch 2 times, most recently from 45d6d82 to 478ec68 Compare November 19, 2020 19:05

andrewballantyne suggested changes Nov 20, 2020

View reviewed changes

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 21, 2020

jeff-phillips-18 force-pushed the topology-package branch from 478ec68 to 59dad45 Compare November 23, 2020 14:17

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 23, 2020

jeff-phillips-18 force-pushed the topology-package branch from 59dad45 to c6fb7a2 Compare November 23, 2020 15:19

andrewballantyne approved these changes Nov 23, 2020

View reviewed changes

openshift-ci-robot assigned andrewballantyne Nov 23, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 23, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 23, 2020

Move topology from dev-console to its own monorepo

6062e7e

jeff-phillips-18 force-pushed the topology-package branch from c6fb7a2 to 6062e7e Compare November 23, 2020 18:46

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 23, 2020

andrewballantyne approved these changes Nov 23, 2020

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 23, 2020

openshift-merge-robot merged commit 81c9159 into openshift:master Nov 24, 2020

spadgett added this to the v4.7 milestone Nov 30, 2020

jeff-phillips-18 deleted the topology-package branch December 2, 2020 13:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move topology from dev-console to its own monorepo #7254

Move topology from dev-console to its own monorepo #7254

jeff-phillips-18 commented Nov 18, 2020

jeff-phillips-18 commented Nov 18, 2020

jeff-phillips-18 commented Nov 18, 2020

andrewballantyne Nov 20, 2020

andrewballantyne Nov 20, 2020

jeff-phillips-18 Nov 20, 2020

andrewballantyne Nov 20, 2020

andrewballantyne Nov 20, 2020

andrewballantyne Nov 20, 2020

andrewballantyne Nov 20, 2020

andrewballantyne Nov 20, 2020

andrewballantyne Nov 20, 2020

andrewballantyne Nov 20, 2020

andrewballantyne Nov 20, 2020

jeff-phillips-18 Nov 23, 2020

andrewballantyne Nov 23, 2020

andrewballantyne Nov 20, 2020

andrewballantyne left a comment

andrewballantyne Nov 23, 2020

andrewballantyne Nov 23, 2020

jeff-phillips-18 Nov 23, 2020

andrewballantyne Nov 23, 2020

andrewballantyne Nov 23, 2020

andrewballantyne commented Nov 23, 2020

christianvogt commented Nov 23, 2020

andrewballantyne left a comment

openshift-ci-robot commented Nov 23, 2020

	import { NamespaceBarApplicationSelector } from '@console/topology/src';
	import { NamespaceBarApplicationSelector } from '@console/topology';

		{!hideApplications && <ApplicationSelector disabled={disabled} />}
		{!hideApplications && <NamespaceBarApplicationSelector disabled={disabled} />}

	import { UNASSIGNED_KEY } from '@console/topology/src/const';
	import { UNASSIGNED_KEY } from '@console/topology';

		@@ -1,4 +1,6 @@
		import { STORAGE_PREFIX } from '@console/shared';
		import { STORAGE_PREFIX } from '@console/shared/src/constants/common';

	import { TopologyViewType } from '@console/topology/src/topology-types';
	import { TopologyViewType } from '@console/topology';


		describe('TopologySideBar:', () => {
		const props: TopologySideBarProps = {

Move topology from dev-console to its own monorepo #7254

Move topology from dev-console to its own monorepo #7254

Conversation

jeff-phillips-18 commented Nov 18, 2020

jeff-phillips-18 commented Nov 18, 2020

jeff-phillips-18 commented Nov 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewballantyne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewballantyne commented Nov 23, 2020

christianvogt commented Nov 23, 2020

andrewballantyne left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Nov 23, 2020