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

MGMT-15374: Unties common-to-ocm dependencies #2279

Merged
merged 2 commits into from
Jul 27, 2023

Conversation

jkilzi
Copy link
Collaborator

@jkilzi jkilzi commented Jul 26, 2023

Basically I used this regex: import (.+) from '.+(?=(cim|ocm)).+'; in order to search for imports referencing the cim or ocm folder from within the common folder.

The search yielded 2 modules:

  • /lib/components/hosts/MassChangeHostnameModal.tsx
  • /lib/components/clusterDetail/KubeconfigDownload.tsx

Once the common folder doesn't depend on ocm or cim, I'll be able to convert the directory into a stand-alone package that ocm and cim can use as a dependency. This will later allow for splitting cim and ocm into different packages.

The PR is a bit large mainly because after moving the cross-dependencies to the common folder I encountered some circular dependencies related to fileSize and stringToJSON. This issue is resolved in the second commit.

@jkilzi jkilzi requested a review from a team as a code owner July 26, 2023 12:58
@openshift-ci openshift-ci bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 26, 2023
@jkilzi jkilzi changed the title Breaks common-to-ocm dependencies MGMT-15374: Breaks common-to-ocm dependencies Jul 26, 2023
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 26, 2023
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 26, 2023

@jkilzi: This pull request references MGMT-15374 which is a valid jira issue.

In response to this:

Basically I used this regex: import (.+) from '.+(?=(cim|ocm)).+'; in order to search for imports referencing the cim or ocm folder from within the common folder.
The search yielded 2 modules:

  • /lib/components/hosts/MassChangeHostnameModal.tsx
  • /lib/components/clusterDetail/KubeconfigDownload.tsx

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.

@jkilzi jkilzi marked this pull request as draft July 26, 2023 13:14
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 26, 2023
@jkilzi jkilzi marked this pull request as ready for review July 26, 2023 13:15
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 26, 2023
@jkilzi jkilzi changed the title MGMT-15374: Breaks common-to-ocm dependencies MGMT-15374: Unties common-to-ocm dependencies Jul 26, 2023
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 26, 2023

@jkilzi: This pull request references MGMT-15374 which is a valid jira issue.

In response to this:

Basically I used this regex: import (.+) from '.+(?=(cim|ocm)).+'; in order to search for imports referencing the cim or ocm folder from within the common folder.

The search yielded 2 modules:

  • /lib/components/hosts/MassChangeHostnameModal.tsx
  • /lib/components/clusterDetail/KubeconfigDownload.tsx

Once the common folder doesn't depend on ocm or cim, I'll be able to convert the directory into a stand-alone package that ocm and cim can use as a dependency. This will later allow for splitting cim and ocm into different packages.

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.

Moves fileSize and stringToJson to lib/common/utils.ts in order to break circular dependencies
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 26, 2023

@jkilzi: This pull request references MGMT-15374 which is a valid jira issue.

In response to this:

Basically I used this regex: import (.+) from '.+(?=(cim|ocm)).+'; in order to search for imports referencing the cim or ocm folder from within the common folder.

The search yielded 2 modules:

  • /lib/components/hosts/MassChangeHostnameModal.tsx
  • /lib/components/clusterDetail/KubeconfigDownload.tsx

Once the common folder doesn't depend on ocm or cim, I'll be able to convert the directory into a stand-alone package that ocm and cim can use as a dependency. This will later allow for splitting cim and ocm into different packages.

The PR is a bit large mainly because after moving the cross-dependencies to the common folder I encountered some circular dependencies related to fileSize and stringToJSON. This issue is resolved in the second commit.

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 lgtm Indicates that a PR is ready to be merged. label Jul 27, 2023
@openshift-ci
Copy link

openshift-ci bot commented Jul 27, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ammont82, jgyselov, jkilzi

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

@jkilzi jkilzi merged commit 8f1d107 into openshift-assisted:master Jul 27, 2023
8 checks passed
@jkilzi jkilzi deleted the tasks/MGMT-15374 branch July 27, 2023 07:37
rawagner pushed a commit to rawagner/facet-lib that referenced this pull request Sep 13, 2023
* Breaks common-to-ocm dependencies

* Breaks circular dependencies

Moves fileSize and stringToJson to lib/common/utils.ts in order to break circular dependencies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants