feat: adding missing english translations for other git providers#1962
feat: adding missing english translations for other git providers#1962WilcoSp wants to merge 10 commits intonpmx-dev:mainfrom
Conversation
…llback & created a composable to get the correct translation. fixed that package page only said "view on github"
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
|
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:
📝 WalkthroughWalkthroughAdds a new composable Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 38b8a77f-e181-4a0a-968c-1fb505432735
📒 Files selected for processing (32)
app/composables/useViewOnGitProvider.tsapp/pages/org/[org].vueapp/pages/package/[[org]]/[name].vueapp/pages/~[username]/index.vuei18n/locales/ar.jsoni18n/locales/az-AZ.jsoni18n/locales/bg-BG.jsoni18n/locales/bn-IN.jsoni18n/locales/cs-CZ.jsoni18n/locales/de-DE.jsoni18n/locales/en.jsoni18n/locales/es.jsoni18n/locales/fr-FR.jsoni18n/locales/hi-IN.jsoni18n/locales/hu-HU.jsoni18n/locales/id-ID.jsoni18n/locales/it-IT.jsoni18n/locales/ja-JP.jsoni18n/locales/kn-IN.jsoni18n/locales/mr-IN.jsoni18n/locales/nb-NO.jsoni18n/locales/ne-NP.jsoni18n/locales/pl-PL.jsoni18n/locales/pt-BR.jsoni18n/locales/ru-RU.jsoni18n/locales/ta-IN.jsoni18n/locales/te-IN.jsoni18n/locales/tr-TR.jsoni18n/locales/uk-UA.jsoni18n/locales/zh-CN.jsoni18n/locales/zh-TW.jsoni18n/schema.json
| "error": "Error" | ||
| "error": "Error", | ||
| "view_on": { | ||
| "npm": "view on npm", |
There was a problem hiding this comment.
Inconsistent capitalisation: "view" should be "View".
All other entries in view_on start with uppercase "View on …", but the npm entry uses lowercase "view on npm". For UI consistency, capitalise "View" while keeping "npm" lowercase per its branding.
✏️ Proposed fix
- "npm": "view on npm",
+ "npm": "View on npm",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "npm": "view on npm", | |
| "npm": "View on npm", |
There was a problem hiding this comment.
it's "view" instead of "View" because it was originally like that, the same for github it's "View" because before moving the translation it also was with "View"
There was a problem hiding this comment.
@WilcoSp, understood — the casing is intentional, preserving the original values from before the keys were moved. Thanks for the context!
✏️ Learnings added
Learnt from: WilcoSp
Repo: npmx-dev/npmx.dev PR: 1962
File: i18n/locales/en.json:194-194
Timestamp: 2026-03-06T16:08:13.589Z
Learning: In `i18n/locales/en.json`, the `common.view_on.npm` entry intentionally uses lowercase "view on npm" (matching its original casing from `common.view_on_npm`), while `common.view_on.github` uses "View on GitHub" (uppercase, matching its original casing from `package.readme.view_on_github`). The inconsistency is deliberate and should not be flagged in future reviews.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: serhalp
Repo: npmx-dev/npmx.dev PR: 1183
File: lunaria/files/uk-UA.json:305-318
Timestamp: 2026-02-08T18:44:10.538Z
Learning: In the npmx.dev project, new or changed entries in i18n translation files (locale JSON files) may be omitted from non-English languages. Translations are not completed in-band in the same PR and are tracked elsewhere. It is acceptable for non-English locale files to be missing keys that exist in English locale files.
Learnt from: userquin
Repo: npmx-dev/npmx.dev PR: 1096
File: i18n/locales/es-419.json:34-41
Timestamp: 2026-02-06T14:53:23.339Z
Learning: In the npmx.dev project, when using country locale variants (e.g., es-419, es-ES), place only translations that differ from the base language in variant JSON files (e.g., es-419.json). The base file (es.json) is loaded first, then the variant file overlays any keys it defines. This behavior is provided by vue-i18n's multiple-files feature. See CONTRIBUTING.md under 'Country variants (advanced)' for details.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| case 'tangled': | ||
| return t('common.view_on.tangled') | ||
| } | ||
| return t('common.view_on.git_repo') |
There was a problem hiding this comment.
If we have string & {}, can we use te fromuseI18n to check if the key exists? Otherwise return gihub
There was a problem hiding this comment.
return uProvider && te(`common.view_on.${uProvider}`) ? t(`common.view_on.${uProvider}`) : t('common.view_on.git_repo')There was a problem hiding this comment.
I'll change it to use te but I do think it's best to return "Git repository" instead of Github as it is closer to the truth than returning github by default
There was a problem hiding this comment.
Add the case to the swtich (without default), then add console.warn (add eslint rule to ignore that console.warn) and return the value
There was a problem hiding this comment.
I've figured out that the keys can also be referenced in comments, that way 'te' can still be used
… into fix/missing_view_on_git
…_on.{provider}"
This reverts commit 145cd00.
…s to satisfy i18n report
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/composables/useViewOnGitProvider.ts (1)
23-34: The comment-only provider list is easy to forget to update.If the i18n report needs static references, consider moving these provider ids into a small shared constant or fixture. That gives you one source of truth instead of a third hard-coded copy alongside the locale/schema entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 958f2467-3139-42db-aecf-97866c4cf639
📒 Files selected for processing (1)
app/composables/useViewOnGitProvider.ts
🔗 Linked comment
Resolved the issue I had raised in this comment & this discord message
🧭 Context
Currently at the package page when a package doesn't have a readme.md the text "No README available." with a link that has the text "View on GitHub" but without any checks what the git provider is & the translations for the git providers are missing.
📚 Description
I've moved the translation of "View on GitHub" from "package.readme.view_on_github" to "common_view_on.github" & created the English translations for the other git providers + a fallback to "common.view_on"
I've also created the composable "useViewOnGitProvider" which will give the "common.view_on" translation based on the given git provider.
I've also changed the key for "common.view_on_npm" to "common.view_on.npm" because of the added translations
I've chosen to have the translations at "common.view_on.*" instead of "package.readme.view_on.*" because for changelog/releases these translations are also needed for a button at the top right that navigates to the changelog.md or releases, here examples with Vue & Nuxt