-
Notifications
You must be signed in to change notification settings - Fork 119
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
VPN-5138: Fix pascal case #9949
Conversation
14759a5
to
947fd91
Compare
72c5a67
to
3d3823c
Compare
3d3823c
to
e77f669
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
R+
} | ||
} | ||
|
||
return words.join(""); | ||
} | ||
|
||
QString toPascalCase(const QString& s) { return changeCasing(s, true); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be helpful to add a comment for our future selves and any poor souls who come after us about when we expect to use toPascalCase vs toLanguageId.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, thanks. Added comments.
Description
servers.Mcallen
, which later converts intoserversMcallen
serversMcAllen
- we weren’t handling capitalization consistently!id = city.split(",")[0].strip().title().replace(" ", “”)
QString i18nCityId = QString("Servers%1").arg(toPascalCase(parsedCityName));
toPascalCase
had a bug:words[i] = word.at(0).toUpper() + word.mid(1)
(bug introduced here: VPN-5175 - Get languages, currencies and server name translations from the l10n repository strings #9448)There is additional bug with
Malmö
that is part of VPN-5138. This bug is Windows-only. I tried debugging via additional logging in the build (you'll see a bunch of commits related to this, as I needed it to run on TaskCluster), but kept hitting dead ends. I do not have a Windows dev machine, and this would be much easier to debug with one. I've filed this part as https://mozilla-hub.atlassian.net/browse/VPN-6649.Reference
VPN-5138
Checklist