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

CONSOLE-3654: console-plugin-template repository should support localization #47

Conversation

Mylanos
Copy link
Contributor

@Mylanos Mylanos commented Jun 22, 2023

Links to: https://issues.redhat.com/browse/CONSOLE-3654

Adds possibility to generate locales in the console-plugin-template repo.

@openshift-ci-robot
Copy link

openshift-ci-robot commented Jun 22, 2023

@Mylanos: This pull request references CONSOLE-3654 which is a valid jira issue.

In response to this:

Links to: https://issues.redhat.com/browse/CONSOLE-3654

Adds possibility to generate locales in the console-plugin-template repo.

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-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 22, 2023
@Mylanos
Copy link
Contributor Author

Mylanos commented Jun 23, 2023

/retest-required

@jhadvig
Copy link
Member

jhadvig commented Jun 23, 2023

  1) Console plugin template test
       "before all" hook for "Verify the example page title":
     CypressError: `cy.exec('cd ../../console-plugin-template && /tmp/helm upgrade -i console-plugin-template charts/openshift-console-plugin -n console-plugin-template --create-namespace --set plugin.image=registry.build01.ci.openshift.org/ci-op-q9b5jsxw/pipeline@sha256:619b1c4b96723afa7f5e21994f8c8c2abf7ca8450b508a07aee2a9aeac392061')` timed out after waiting `60000ms`.
https://on.cypress.io/exec
Because this error occurred during a `before all` hook we are skipping the remaining tests in the current suite: `Console plugin template test`
Although you have test retries enabled, we do not retry tests when `before all` or `after all` hooks fail

/retest

@Mylanos
Copy link
Contributor Author

Mylanos commented Jul 17, 2023

/retest

1 similar comment
@jhadvig
Copy link
Member

jhadvig commented Jul 18, 2023

/retest

…that gets called with the title text instead of static string before changes. Changed the name of the locale file so that it follows plugin__<name-of-the-plugin> convention. Added a test-frontend.sh script that checks if the strings in the plugin have been localized using i18n.
…to the build directory, reverted should(include.text) to original should(contain) in the example-page-cy.ts test as it didnt cause any problems in the end
@Mylanos
Copy link
Contributor Author

Mylanos commented Aug 4, 2023

{Operator unavailable (null): operator is not reporting conditions Operator unavailable (null): operator is not reporting conditions}

{ openshift cluster install failed with cluster creation}

{ openshift cluster install failed overall}

@Mylanos
Copy link
Contributor Author

Mylanos commented Aug 4, 2023

/retest

@Mylanos
Copy link
Contributor Author

Mylanos commented Aug 4, 2023

/retest

Copy link
Member

@jhadvig jhadvig left a comment

Choose a reason for hiding this comment

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

One small nit, otherwise good to go 👍

README.md Outdated
```

NOTE: When deploying on OpenShift 4.10, it is recommended to add the parameter `--set plugin.securityContext.enabled=false` which will omit configurations related to Pod Security.

NOTE: Please adhere to the specified namespace format `plugin__<name-of-the-plugin>` when defining namespaces. The name of the plugin should be extracted from the `consolePlugin` declaration within the [package.json](package.json) file.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
NOTE: Please adhere to the specified namespace format `plugin__<name-of-the-plugin>` when defining namespaces. The name of the plugin should be extracted from the `consolePlugin` declaration within the [package.json](package.json) file.
NOTE: When defining i18n namespace, adhere `plugin__<name-of-the-plugin>` format. The name of the plugin should be extracted from the `consolePlugin` declaration within the [package.json](package.json) file.

@openshift-ci
Copy link

openshift-ci bot commented Aug 10, 2023

@Mylanos: all tests passed!

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.

Copy link
Member

@jhadvig jhadvig left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@jhadvig
Copy link
Member

jhadvig commented Aug 10, 2023

QE Approver:
/assign @yanpzhan

Docs Approver:
/assign @opayne1

PX Approver:
/assign @RickJWagner

@jhadvig jhadvig added approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. labels Aug 10, 2023
@openshift-ci
Copy link

openshift-ci bot commented Aug 10, 2023

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: jhadvig, Mylanos

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

@RickJWagner
Copy link

/label px-approved

@RickJWagner RickJWagner removed their assignment Aug 10, 2023
@openshift-ci openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Aug 10, 2023
@opayne1
Copy link

opayne1 commented Aug 10, 2023

/label docs-approved

@openshift-ci openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Aug 10, 2023
@yanpzhan
Copy link

I run the plugin using a local development environment by:
In one terminal window, run:
yarn install
yarn run start
In another terminal window, run:
oc login
yarn run start-console
Then access "localhost:9000/example?pseudolocalization=true&lng=en", could see contents are marked for i18n.
Screenshot from 2023-08-14 17-46-49
@Mylanos If there others I need to check for the US?

@Mylanos
Copy link
Contributor Author

Mylanos commented Aug 14, 2023

I run the plugin using a local development environment by: In one terminal window, run: yarn install yarn run start In another terminal window, run: oc login yarn run start-console Then access "localhost:9000/example?pseudolocalization=true&lng=en", could see contents are marked for i18n. Screenshot from 2023-08-14 17-46-49 @Mylanos If there others I need to check for the US?

I'm sorry but I'm not sure if I understand correctly, could you please clarify the question?

@yanpzhan
Copy link

@Mylanos I mean except the check in above screenshot , if there are any other checkpoints I need to test for the feature from the pr code.

@Mylanos
Copy link
Contributor Author

Mylanos commented Aug 15, 2023

@Mylanos I mean except the check in above screenshot , if there are any other checkpoints I need to test for the feature from the pr code.

That's pretty much it, other than that the features that this PR brings is the locale/translations files and contents get automatically generated from source codes via yarn i18n, so you can check if all the strings inside the source code get translated after you remove the locale files. But that is closely tied with the check if all the strings on the example page are marked for i18n.

@yanpzhan
Copy link

yanpzhan commented Aug 16, 2023

@Mylanos Thanks. I tried to remove file:
# rm locales/en/plugin__console-plugin-template.json
# yarn i18n
After yarn i18n, the file under locals is generated again.
/label qe-approved

@openshift-ci
Copy link

openshift-ci bot commented Aug 16, 2023

@yanpzhan: The label(s) /label qe-approved. cannot be applied. These labels are supported: platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, downstream-change-needed, rebase/manual, approved, backport-risk-assessed, bugzilla/valid-bug, cherry-pick-approved, jira/valid-bug, staff-eng-approved. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

@Mylanos Thanks. I tried to remove file:
# rm locales/en/plugin__console-plugin-template.json
# yarn i18n
After yarn i18n, the file under locals is generated again.
/label qe-approved.

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.

@yanpzhan
Copy link

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Aug 16, 2023
@openshift-merge-robot openshift-merge-robot merged commit d2d200b into openshift:main Aug 16, 2023
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. docs-approved Signifies that Docs has signed off on this PR 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. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants