Skip to content

Make sure i18n keys are consistent#1522

Merged
zachmargolis merged 1 commit intomasterfrom
margolis-clean-up-i18n-keys
Jul 3, 2017
Merged

Make sure i18n keys are consistent#1522
zachmargolis merged 1 commit intomasterfrom
margolis-clean-up-i18n-keys

Conversation

@zachmargolis
Copy link
Contributor

Why: Helps with our translation pipeline

How: Make sure keys use word characters and underscores only

Copy link
Contributor

Choose a reason for hiding this comment

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

Any concerns about leading, trailing, or double underscores? \A[a-z0-9]+(_[a-z0-9]+)*\z protects against that, but is not as pretty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm OK not worrying about those cases?

The things that caused us problems in this most recent round were spaces, quotes and dashes in keys, so I just wanted to handle those. A few extra underscores seems ok for now to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ooops, forgot the dot. Get's really ugly once you factor that in. Probs not worth it.

Copy link
Contributor

@monfresh monfresh left a comment

Choose a reason for hiding this comment

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

lgtm

@monfresh
Copy link
Contributor

Looks like there is a spec that needs to be updated to look for the new key name.

@zachmargolis zachmargolis force-pushed the margolis-clean-up-i18n-keys branch from 568bca4 to ecdad0e Compare July 3, 2017 13:13
**Why**: Helps with our translation pipeline

**How**: Make sure keys use word characters and underscores only
@zachmargolis zachmargolis merged commit 9282b5e into master Jul 3, 2017
@zachmargolis zachmargolis deleted the margolis-clean-up-i18n-keys branch July 3, 2017 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants