HELM-763: Fix microscopic helm spelling - #16785
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughHelm release and chart terminology was standardized across English translations, UI components, Cypress support code, and Gherkin integration scenarios. URL installation guidance and related test assertions were also reworded. ChangesHelm copy alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
component/helm
|
/retest |
|
/label docs-approved |
openshift-ci
Bot
added
the
docs-approved
|
i can't tell now what comments i made and whether they were applied, so i'm conditionally approving in good faith. thanks! |
sowmya-sl
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@sowmya-sl: This pull request references HELM-763 which is a valid jira issue. DetailsIn 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
Bot
added
the
needs-rebase
221f691 to
a09dca8
Compare
openshift-ci
Bot
removed
the
needs-rebase
| " (Provided by {{chartRepoName}})": "helm-plugin~ (Provided by {{chartRepoName}})", | ||
| " / App Version {{appVersion}}": "helm-plugin~ / App Version {{appVersion}}", |
There was a problem hiding this comment.
The helm-plugin~ prefix is supposed to appear in the reference (i.e., in the invocation of the t() function), not in the translation string...right?? (The prefix indicates which file to pull from...right?...so it doesn't make sense to have it inside that file....) Note that this problem occurs repeatedly.
Also, it seems a little weird/fragile to have the string start with a blank space...but, I see that you didn't introduce it.
There was a problem hiding this comment.
The helm-plugin~ prefix is added as the output of yarn i18n. Its because source code has helm-plugin~ added - which is a utility file. The reason these utility files use t('helm-plugin~...') instead of just t('...') is that they're plain .ts files that can't call useTranslation('helm-plugin') ( React hook only works in components). The inline helm-plugin~ is how the extractor knows the key belongs to the helm-plugin namespace. Not adding the helm-plugin~ pushes these to public.json
There was a problem hiding this comment.
What you're saying makes sense in the context of a Javascript or Typescript source file. That is, the helm-plugin~ prefix should be used there in the invocations of t() as needed.
But, this file here is the .json file that the prefix refers to. The prefix shouldn't appear inside this file. (In particular, it shouldn't appear in the string translation -- that will be displayed to the user!)
There was a problem hiding this comment.
You're right. Removing this from the file.
| "Latest Chart version": "Latest Chart version", | ||
| "Maintainers": "Maintainers", | ||
| "Must be a valid OCI URL or a valid HTTP/HTTPS tar file; for example - oci://registry.example.com/chart, https://example.com/chart-1.0.0.tgz.": "Must be a valid OCI URL or a valid HTTP/HTTPS tar file; for example - oci://registry.example.com/chart, https://example.com/chart-1.0.0.tgz.", | ||
| "Latest chart version": "helm-plugin~Latest chart version", |
There was a problem hiding this comment.
Is this change correct? Here we are down-casing chart, whereas earlier lines (like 82 & 83) are up-casing it.
There was a problem hiding this comment.
Chart is lowercase when its not preceded by Helm.
webbnh
left a comment
webbnh
left a comment
There was a problem hiding this comment.
(Sorry, I hit "Submit review" accidentally on my previous comments. 😞)
It looks like there are a few problems, here:
- The changes to
Chartlook inconsistent. - There seem to be a number of errant
helm-plugin~insertions. - There's a translation key addition/change that you probably don't want in this PR.
|
Scheduling tests matching the |
|
/lgtm cancel unresolved review comments |
a09dca8 to
2011305
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsx`:
- Line 114: Update the Alert title in HelmURLInstallForm to use lowercase
“chart” in “You cannot install the Helm chart.”
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 777b0ffe-de48-482c-9ce1-86798610e6e2
📒 Files selected for processing (40)
frontend/packages/helm-plugin/integration-tests/features/helm-release.featurefrontend/packages/helm-plugin/integration-tests/features/helm/actions-on-helm-release-after-upgrade.featurefrontend/packages/helm-plugin/integration-tests/features/helm/actions-on-helm-release.featurefrontend/packages/helm-plugin/integration-tests/features/helm/helm-compatibility.featurefrontend/packages/helm-plugin/integration-tests/features/helm/helm-installation-view.featurefrontend/packages/helm-plugin/integration-tests/features/helm/helm-navigation.featurefrontend/packages/helm-plugin/integration-tests/features/helm/helm-page-tabs.featurefrontend/packages/helm-plugin/integration-tests/features/helm/install-helm-chart.featurefrontend/packages/helm-plugin/integration-tests/features/helm/install-url-chart.featurefrontend/packages/helm-plugin/integration-tests/features/helm/topology-helm-release.featurefrontend/packages/helm-plugin/integration-tests/support/constants/static-text/helm-text.tsfrontend/packages/helm-plugin/integration-tests/support/pages/helm/helm-details-page.tsfrontend/packages/helm-plugin/integration-tests/support/pages/helm/helm-page.tsfrontend/packages/helm-plugin/integration-tests/support/pages/helm/upgrade-helm-release-page.tsfrontend/packages/helm-plugin/integration-tests/support/step-definitions/common/common.tsfrontend/packages/helm-plugin/integration-tests/support/step-definitions/helm/helm-compatibility.tsfrontend/packages/helm-plugin/integration-tests/support/step-definitions/helm/helm-navigation.tsfrontend/packages/helm-plugin/integration-tests/support/step-definitions/helm/helm-release.tsfrontend/packages/helm-plugin/locales/en/helm-plugin.jsonfrontend/packages/helm-plugin/src/actions/creators.tsfrontend/packages/helm-plugin/src/catalog/utils/catalog-utils.tsxfrontend/packages/helm-plugin/src/components/__tests__/helm-release-mock-data.tsfrontend/packages/helm-plugin/src/components/details-page/HelmReleaseDetails.tsxfrontend/packages/helm-plugin/src/components/details-page/history/HelmReleaseHistory.tsxfrontend/packages/helm-plugin/src/components/details-page/overview/HelmReleaseOverview.tsxfrontend/packages/helm-plugin/src/components/details-page/overview/__tests__/HelmReleaseOverview.spec.tsxfrontend/packages/helm-plugin/src/components/forms/__tests__/HelmInstallUpgradeForm.spec.tsxfrontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradeForm.tsxfrontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsxfrontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartInstallPage.tsxfrontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLInstallForm.tsxfrontend/packages/helm-plugin/src/components/forms/url-chart/helm-oci-validation-utils.tsfrontend/packages/helm-plugin/src/components/list-page/HelmReleaseList.tsxfrontend/packages/helm-plugin/src/components/list-page/HelmReleaseListPage.tsxfrontend/packages/helm-plugin/src/components/list-page/HelmReleaseListRow.tsxfrontend/packages/helm-plugin/src/components/list-page/HelmTabbedPage.tsxfrontend/packages/helm-plugin/src/models/helm.tsfrontend/packages/helm-plugin/src/topology/helmFilters.tsfrontend/packages/helm-plugin/src/utils/__tests__/helm-utils.spec.tsfrontend/packages/helm-plugin/src/utils/helm-utils.ts
💤 Files with no reviewable changes (1)
- frontend/packages/helm-plugin/integration-tests/support/step-definitions/common/common.ts
🚧 Files skipped from review as they are similar to previous changes (32)
- frontend/packages/helm-plugin/integration-tests/support/pages/helm/upgrade-helm-release-page.ts
- frontend/packages/helm-plugin/integration-tests/support/constants/static-text/helm-text.ts
- frontend/packages/helm-plugin/integration-tests/features/helm/helm-installation-view.feature
- frontend/packages/helm-plugin/integration-tests/features/helm/install-url-chart.feature
- frontend/packages/helm-plugin/integration-tests/features/helm/install-helm-chart.feature
- frontend/packages/helm-plugin/integration-tests/support/pages/helm/helm-page.ts
- frontend/packages/helm-plugin/src/components/list-page/HelmReleaseListRow.tsx
- frontend/packages/helm-plugin/integration-tests/features/helm/topology-helm-release.feature
- frontend/packages/helm-plugin/src/components/list-page/HelmReleaseList.tsx
- frontend/packages/helm-plugin/src/models/helm.ts
- frontend/packages/helm-plugin/src/components/details-page/HelmReleaseDetails.tsx
- frontend/packages/helm-plugin/integration-tests/features/helm/helm-compatibility.feature
- frontend/packages/helm-plugin/src/catalog/utils/catalog-utils.tsx
- frontend/packages/helm-plugin/src/components/forms/tests/HelmInstallUpgradeForm.spec.tsx
- frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartForm.tsx
- frontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmInstallUpgradeForm.tsx
- frontend/packages/helm-plugin/integration-tests/support/pages/helm/helm-details-page.ts
- frontend/packages/helm-plugin/src/components/tests/helm-release-mock-data.ts
- frontend/packages/helm-plugin/integration-tests/features/helm/helm-page-tabs.feature
- frontend/packages/helm-plugin/src/components/details-page/overview/tests/HelmReleaseOverview.spec.tsx
- frontend/packages/helm-plugin/src/components/details-page/history/HelmReleaseHistory.tsx
- frontend/packages/helm-plugin/integration-tests/support/step-definitions/helm/helm-compatibility.ts
- frontend/packages/helm-plugin/integration-tests/features/helm/actions-on-helm-release-after-upgrade.feature
- frontend/packages/helm-plugin/src/utils/helm-utils.ts
- frontend/packages/helm-plugin/src/components/forms/url-chart/HelmURLChartInstallPage.tsx
- frontend/packages/helm-plugin/integration-tests/support/step-definitions/helm/helm-navigation.ts
- frontend/packages/helm-plugin/integration-tests/features/helm/helm-navigation.feature
- frontend/packages/helm-plugin/integration-tests/features/helm/actions-on-helm-release.feature
- frontend/packages/helm-plugin/src/components/list-page/HelmTabbedPage.tsx
- frontend/packages/helm-plugin/src/utils/tests/helm-utils.spec.ts
- frontend/packages/helm-plugin/locales/en/helm-plugin.json
- frontend/packages/helm-plugin/integration-tests/features/helm-release.feature
|
@sowmya-sl: This PR has been marked as verified by DetailsIn 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. |
|
/test e2e-gcp-console |
Update Helm Cypress E2E test selectors, constants, and assertions to
use sentence case ("Helm release", "Helm releases") matching the
current UI labels. The source code was updated to sentence case but
the Cypress tests still referenced title case, causing test failures.
Co-authored-by: Cursor <cursoragent@cursor.com>
openshift-ci-robot
removed
the
verified
openshift-ci
Bot
added
component/dev-console
|
Scheduling tests matching the |
|
/test e2e-gcp-console |
The page heading is "Helm" (from MultiTabListPage), not "Helm releases" which is only the tab label. Update the Cypress step definition to check for the correct heading text. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/lgtm |
openshift-ci
Bot
added
the
tide/merge-method-squash
openshift-ci-robot
added
the
verified
|
@logonoff: This PR has been marked as verified by DetailsIn 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. |
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jseseCCS, logonoff, sowmya-sl, webbnh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
68185f4
into
openshift:main
|
/backport 5.0 |
|
/cherry-pick release-5.0 |
|
@sowmya-sl: new pull request created: #17012 DetailsIn 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 kubernetes-sigs/prow repository. |
6 participants
Change microscopic spelling, active voice changes to maintain consistency.
Additional info:
Based on #16676
Reviewers and assignees:
Summary by CodeRabbit