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.

Sign up for GitHub

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

Jump to bottom

OCPBUGS-44800: Add Dynamic Plugins nav item #14521

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

OCPBUGS-44800: Add Dynamic Plugins nav item #14521

wants to merge 1 commit into from
+37 −1

Conversation

rhamilto
Copy link
Member

@rhamilto rhamilto commented Nov 20, 2024 •
edited
Loading

localhost_9000_k8s_cluster_operator openshift iov1Console_cluster_console-plugins

@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Nov 20, 2024
@openshift-ci-robot
Copy link
Contributor

@rhamilto: This pull request references Jira Issue OCPBUGS-44800, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.18.0) matches configured target version for branch (4.18.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @yapei

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Screenshot 2024-11-20 at 9 39 16 AM

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 added component/core Related to console core functionality approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 20, 2024
@rhamilto rhamilto mentioned this pull request Nov 20, 2024
Merged
1 task
@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 Nov 20, 2024
@rhamilto
Copy link
Member Author

rhamilto commented Nov 20, 2024 •
edited
Loading

We use the label Dynamic plugins in the Overview and the About modal (screenshots below), but I think Console Plugins is the proper choice for the nav item since that aligns with the tab label, but it is odd we have this discrepancy. Maybe we need to revisit? @jhadvig, @spadgett, @vojtechszocs, WDYT?
Screenshot 2024-11-20 at 9 53 05 AM
Screenshot 2024-11-20 at 9 53 15 AM

@rhamilto
Copy link
Member Author

/retest

1 similar comment
@rhamilto
Copy link
Member Author

/retest

@rhamilto
Copy link
Member Author

/test okd-scos-e2e-aws-ovn

@openshift-ci-robot
Copy link
Contributor

@rhamilto: This pull request references Jira Issue OCPBUGS-44800, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.18.0) matches configured target version for branch (4.18.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira ([email protected]), skipping review request.

In response to this:

localhost_9000_k8s_cluster_operator openshift iov1Console_cluster_console-plugins

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.

@rhamilto
Copy link
Member Author

We use the label Dynamic plugins in the Overview and the About modal (screenshots below), but I think Console Plugins is the proper choice for the nav item since that aligns with the tab label, but it is odd we have this discrepancy. Maybe we need to revisit? @jhadvig, @spadgett, @vojtechszocs, WDYT?

In the light of a new day, I realized the nav item label should be Dynamic Plugins, so I've updated this PR.

@rhamilto
Copy link
Member Author

/label acknowledge-critical-fixes-only

@openshift-ci openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Nov 21, 2024
@rhamilto rhamilto changed the title OCPBUGS-44800: Add Console Plugins nav item OCPBUGS-44800: Add Dynamic Plugins nav item Nov 21, 2024
vojtechszocs
vojtechszocs suggested changes Nov 21, 2024
View reviewed changes
frontend/packages/console-app/src/consts.ts Outdated
@@ -5,3 +5,4 @@ export const HIDE_USER_WORKLOAD_NOTIFICATIONS_USER_SETTINGS_KEY =
export const FLAG_DEVELOPER_PERSPECTIVE = 'DEVELOPER_PERSPECTIVE';
export const ACM_PERSPECTIVE_ID = 'acm';
export const ADMIN_PERSPECTIVE_ID = 'admin';
export const FLAG_CAN_GET_CONSOLE_OPERATOR_CONFIG = 'FLAG_CAN_GET_CONSOLE_OPERATOR_CONFIG';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export const FLAG_CAN_GET_CONSOLE_OPERATOR_CONFIG = 'FLAG_CAN_GET_CONSOLE_OPERATOR_CONFIG';
export const FLAG_CAN_GET_CONSOLE_OPERATOR_CONFIG = 'CAN_GET_CONSOLE_OPERATOR_CONFIG';

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, @vojtechszocs. Thanks. Updated.

@vojtechszocs
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 21, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 21, 2024

[APPROVALNOTIFIER] This PR is APPROVED

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

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@spadgett
Copy link
Member

We use the label Dynamic plugins in the Overview and the About modal (screenshots below), but I think Console Plugins is the proper choice for the nav item since that aligns with the tab label, but it is odd we have this discrepancy. Maybe we need to revisit? @jhadvig, @spadgett, @vojtechszocs, WDYT?

IMO it should be "Console Plugins" everywhere. That's the only kind of plugin we've ever had an external API for. "Static plugins" were always an internal thing, so there's not a need to differentiate between them for users. I think putting "Dynamic" in the label raises more questions than it answers for most users. Also "Console Plugin" matches the CRD kind.

@rhamilto
Copy link
Member Author

rhamilto commented Nov 21, 2024 •
edited
Loading

We use the label Dynamic plugins in the Overview and the About modal (screenshots below), but I think Console Plugins is the proper choice for the nav item since that aligns with the tab label, but it is odd we have this discrepancy. Maybe we need to revisit? @jhadvig, @spadgett, @vojtechszocs, WDYT?

IMO it should be "Console Plugins" everywhere. That's the only kind of plugin we've ever had an external API for. "Static plugins" were always an internal thing, so there's not a need to differentiate between them for users. I think putting "Dynamic" in the label raises more questions than it answers for most users. Also "Console Plugin" matches the CRD kind.

IIRC, we went with Dynamic Plugins so as not to conflate the functionality with the resource. One could argue that subtlety is lost on users however...

Docs uses "Dynamic Plugins": https://docs.openshift.com/container-platform/4.17/web_console/dynamic-plugin/overview-dynamic-plugin.html

@spadgett
Copy link
Member

IIRC, we went with Dynamic Plugins so as not to conflate the functionality with the resource. One could argue that subtlety is lost on users however...

To me, this could create more confusion because it sounds like they're two different things. There are also other kinds of plugins on the platform (for instance, network plugins or CLI plugins). The most important differentiator is that it's a console plugin.

Docs uses "Dynamic Plugins": https://docs.openshift.com/container-platform/4.17/web_console/dynamic-plugin/overview-dynamic-plugin.html

I'd update the docs as well to use "Console Plugins."

@rhamilto
Copy link
Member Author

rhamilto commented Nov 21, 2024 •
edited
Loading

I added a story for 4.19.

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD ea76ee6 and 2 for PR HEAD 16c6473 in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD d878879 and 1 for PR HEAD 16c6473 in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD d878879 and 2 for PR HEAD 16c6473 in total

3 similar comments
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD d878879 and 2 for PR HEAD 16c6473 in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD d878879 and 2 for PR HEAD 16c6473 in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD d878879 and 2 for PR HEAD 16c6473 in total

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 25, 2024

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

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD d878879 and 2 for PR HEAD 16c6473 in total

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vojtechszocs vojtechszocs vojtechszocs requested changes

@yapei yapei Awaiting requested review from yapei

@cyril-ui-developer cyril-ui-developer Awaiting requested review from cyril-ui-developer

@spadgett spadgett Awaiting requested review from spadgett

Assignees

@vojtechszocs vojtechszocs

Labels
acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. 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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rhamilto @openshift-ci-robot @vojtechszocs @spadgett