Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T7046: Add a linter rule for detecting missing variables for `pht`
- Commits
- rARC17ec3859cd36: Fix `pht` method calls
arc lint
Diff Detail
- Repository
- rARC Arcanist
- Branch
- master
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 4337 Build 4350: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
One inline, let me know if that's clear? These might also have translations specified already that could need to be cleaned up.
| src/workflow/ArcanistCommitWorkflow.php | ||
|---|---|---|
| 215–217 | These aren't wrong, they just aren't in the correct form. The intent is to allow translations to choose pluralization, but the way this was done (likely by me) is very confusing because it doesn't use the parameter in the string. This technically works (translations can make alternate-text decisions based on extra parameters), it's just super derpy and we/I stopped doing this a long time ago. Better would be to include the count in the string: %s locally modified path(s) are not included in this revision: ...then make the translations, not the original text, omit the number in English: A locally modified path is not included in this revision: Locally modified paths are not included in this revision: This style is better in every way, and especially better now that you've built lint for it, so this doesn't create any kind of special case that we need to think about, lint-wise. | |
Sounds reasonable. Where are the base translations for these strings anyway? PhabricatorBaseEnglishTranslation lives in rP.