Skip to content

Conversation

@logonoff
Copy link
Member

@logonoff logonoff commented Oct 17, 2025

Note: the cycles produced by the frontend test are NOT as a result of new code introduced in this PR.

When we check for import cycles in the frontend test, we exclude any cycle with get-active-plugins.js. This is because there were a lot of cycles that were very difficult to fix due to the structure of how the console PluginStore is loaded.
Thus, any import cycle which includes the active plugins module doesn't count as a cycle in our eyes.

Meanwhile, each plugin entry point plugin.tsx is imported by the code produced by get-active-plugins.js. Now that we are trying to remove plugin.tsx from console-app, we are uncovering a bunch of import cycles that were previously not counted as get-active-plugins.js was previously in the cycle. Now that the cycles no longer include get-active-plugins, we are counting them.

Reverting only the changes to plugin.tsx shows this -- the detected cycles no longer appear.

Due to the amount of cycles that have to be dealt with (over 200), I want to address this in future work.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 17, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 17, 2025

@logonoff: This pull request references CONSOLE-4837 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

Details

In 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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot requested review from rhamilto and sg00dwin October 17, 2025 17:40
@openshift-ci openshift-ci bot added component/core Related to console core functionality approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 17, 2025
@logonoff logonoff force-pushed the CONSOLE-4837-s2e8-performance-review branch from 3335838 to ce0c4e0 Compare October 17, 2025 17:41
@openshift-ci openshift-ci bot added the kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated label Oct 17, 2025
@logonoff
Copy link
Member Author

/label px-approved
/label docs-approved

@openshift-ci openshift-ci bot added px-approved Signifies that Product Support has signed off on this PR docs-approved Signifies that Docs has signed off on this PR labels Oct 17, 2025
@logonoff logonoff force-pushed the CONSOLE-4837-s2e8-performance-review branch 3 times, most recently from f471f26 to 79a9066 Compare October 18, 2025 01:15
@openshift-ci openshift-ci bot added component/dev-console Related to dev-console component/knative Related to knative-plugin component/shared Related to console-shared component/topology Related to topology labels Oct 18, 2025
@logonoff
Copy link
Member Author

logonoff commented Oct 20, 2025

/hold

I am stopped by my own tests...

@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 Oct 20, 2025
@logonoff logonoff force-pushed the CONSOLE-4837-s2e8-performance-review branch 3 times, most recently from 7ce9b1e to 0fd40ff Compare October 20, 2025 19:47
@logonoff
Copy link
Member Author

image

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 20, 2025
@logonoff
Copy link
Member Author

/retest

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 23, 2025

@logonoff: This pull request references CONSOLE-4837 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

Details

In response to this:

Note: the cycles produced by the frontend test are NOT as a result of new code introduced in this PR.

When we check for import cycles in the frontend test, we exclude any cycle with get-active-plugins.js. This is because there were a lot of cycles that were very difficult to fix due to the structure of how the console PluginStore is loaded.
Thus, any import cycle which includes the active plugins module doesn't count as a cycle in our eyes.

Meanwhile, each plugin entry point plugin.tsx is imported by the code produced by get-active-plugins.js. Now that we are trying to remove plugin.tsx from console-app, we are uncovering a bunch of import cycles that were previously not counted as get-active-plugins.js was previously in the cycle. Now that the cycles no longer include get-active-plugins, we are counting them.

Reverting only the changes to plugin.tsx shows this -- the detected cycles no longer appear.

Due to the amount of cycles that have to be dealt with (over 200), I want to address this in future work.

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 openshift-eng/jira-lifecycle-plugin repository.

@logonoff
Copy link
Member Author

/assign @yapei @vojtechszocs

@logonoff logonoff force-pushed the CONSOLE-4837-s2e8-performance-review branch 3 times, most recently from 37b307f to 6df6102 Compare October 23, 2025 17:04
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 23, 2025
@logonoff logonoff force-pushed the CONSOLE-4837-s2e8-performance-review branch from 6df6102 to 9873d1f Compare October 24, 2025 03:01
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 24, 2025
@logonoff logonoff force-pushed the CONSOLE-4837-s2e8-performance-review branch 4 times, most recently from af19e81 to 04ce49e Compare October 24, 2025 20:01
Note: the cycles produced by the frontend test are NOT as a result of new code introduced in this PR.

When we check for import cycles in the frontend test, we exclude any cycle with `get-active-plugins.js`. This is because there were a lot of cycles that were very difficult to fix due to the structure of how the console `PluginStore` is loaded.
Thus, any import cycle which includes the active plugins module doesn't count as a cycle in our eyes.

Meanwhile, each plugin entry point `plugin.tsx` is imported by the code produced by `get-active-plugins.js`. Now that we are trying to remove `plugin.tsx` from `console-app,` we are uncovering a bunch of import cycles that were previously not counted as `get-active-plugins.js` was previously in the cycle. Now that the cycles no longer include `get-active-plugins`, we are counting them.

Reverting only the changes to `plugin.tsx` shows this -- the detected cycles no longer appear.

Due to the amount of cycles that have to be dealt with (over 200), I want to address this in future work.

Claude prompt for future me:

This project currently has a bunch of import cycles. Import cycles can be detected by running "yarn check-cycles" in the `frontend` directly of this repository, and reading `frontend/.webpack-cycles`.

Your job is to fix all of the import cycles so that `yarn check-cycles` exits with code 0 instead of 1. You will fix cycles by rewriting imports--good examples can be observed in the last git commit, "CONSOLE-4837: Fix cycles". You will rewrite the imports by replacing their "index" or "barrel" file imports with the direct equivalent.

After rewriting imports, run `yarn check-cycles` again to confirm that all cycles have been fixed. Repeat until there are no more import cycles.
@logonoff logonoff force-pushed the CONSOLE-4837-s2e8-performance-review branch from 04ce49e to ce12a9e Compare October 27, 2025 04:04
@XiyunZhao
Copy link
Contributor

/verified by @XiyunZhao
no issue found on the regression testing, and the checkpoint below has been checked

  • The labels on the resource list page are displayed correctly
  • The title on the resource details page is displayed correctly
  • Abbreviations work properly in search and filter functions
  • The breadcrumb navigation is displayed correctly
  • No console errors or warnings are present
  • No errors occur during the build process
  • No API error found on the browser console

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Oct 27, 2025
@openshift-ci-robot
Copy link
Contributor

@XiyunZhao: This PR has been marked as verified by @XiyunZhao.

Details

In response to this:

/verified by @XiyunZhao
no issue found on the regression testing, and the checkpoint below has been checked

  • The labels on the resource list page are displayed correctly
  • The title on the resource details page is displayed correctly
  • Abbreviations work properly in search and filter functions
  • The breadcrumb navigation is displayed correctly
  • No console errors or warnings are present
  • No errors occur during the build process
  • No API error found on the browser console

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 openshift-eng/jira-lifecycle-plugin repository.

@vojtechszocs
Copy link
Contributor

/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 27, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: logonoff, vojtechszocs

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

The pull request process is described here

Details 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

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 27, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 27, 2025

@logonoff: all tests passed!

Full PR test history. Your PR dashboard.

Details

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

@openshift-merge-bot openshift-merge-bot bot merged commit c039d78 into openshift:main Oct 27, 2025
8 checks passed
@logonoff logonoff deleted the CONSOLE-4837-s2e8-performance-review branch October 27, 2025 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality component/dev-console Related to dev-console component/knative Related to knative-plugin component/shared Related to console-shared component/topology Related to topology docs-approved Signifies that Docs has signed off on this PR hacktoberfest-accepted jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants