Setup Legacy Localization Support

[andrewdmontgomery]

Closes #339

Description

This sets up localization of the SDK and Demo apps:

• Configures the SDK Package to support localization
• Configures the GravatarUI target with a Localizable.strings file
• Updates Fastlane and the Makefile to allow for automated generation of strings from source code

Note: this is just basic support for localization

This only introduces support for localization. Notably:

• This does not integrate with GlotPress
• The existing Makefile/Fastlane functionality is there for demonstration purposes only. Once we have decided on a localization flow, this part will change considerably. But the basic functionality will be the same: the content of the Localizable.strings files will be maintained by automation, not manually.

Testing Steps

Update an SDK source file and generate strings

1. Find a source file with a localizable string
2. Modify it:
   • Add or modify a comment
   • Modify a key
   • Modify a value
3. Update the Localizable.strings file using the command line: make generate-strings

• [ ] OBSERVE: you should see your change appear in Sources/GavatarUI/Resources/en.lproj/Localizable.strings

Update a Demo source file and generate strings

Right now, only one string is localizable. It's the Text("Email:") view in DemoAvatarPickerView. For this test, you can either modify that one, or (if you're familiar) add localization support to one of other views.

1. Find a source file with a localizable string
2. Modify it:

• Add or modify a comment
• Modify a key
• Modify a value

3. Update the Localizable.strings file using the command line: make generate-strings

• [ ] OBSERVE: you should see your change appear in Sources/GavatarUI/Resources/en.lproj/Localizable.strings

[wpmobilebot]

📲 You can test the changes from this Pull Request in Gravatar SwiftUI Prototype Build by scanning the QR code below to install the corresponding build.

	App Name	Gravatar SwiftUI Prototype Build
	Build Number	1142
	Version	1.0
	Bundle ID	com.automattic.gravatar-sdk-demo-swiftui.prototype-build
	Commit	c5c550a263b8206722640f23a32515771859cfe8
	App Center Build	Gravatar SDK Demo - SwiftUI #66

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

[wpmobilebot]

📲 You can test the changes from this Pull Request in Gravatar UIKit Prototype Build by scanning the QR code below to install the corresponding build.

	App Name	Gravatar UIKit Prototype Build
	Build Number	1142
	Version	1.0
	Bundle ID	com.automattic.gravatar-sdk-demo-uikit.prototype-build
	Commit	c5c550a263b8206722640f23a32515771859cfe8
	App Center Build	Gravatar SDK Demo - UIKit #67

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

[andrewdmontgomery]

I want to call out one change especially:

Sources/GravatarUI/SwiftUI/AvatarPicker/AvatarPickerView.swift

Right now, the private enum Localized {...} uses some nested enum to structure these string references. I also tried this with a flat enum Localized, too, with entries like:

• static let contentLoadingSuccessTitle = ...
• static let contentLoadingSuccessSubtext = ...
• static let contentLoadingSessionExpiredTitle = ...
• static let contentLoadingSessionExpiredSubtitle = ...

And then calling:

• title: Localized.contentLoadingSuccessTitle
• title: Localized.contentLoadingSuccessSubtext
• title: Localized.contentLoadingSessionExpiredTitle
• title: Localized.contentLoadingSessionExpiredSubtext

But with nesting, I found it a bit easier to keep track of everything:

• Easier to reason within the Localized enum
• Easier to reason from the call site

I find that the calling site is easier to read this way:

• title: Localized.ContentLoading.Success.title
• title: Localized.ContentLoading.Success.subtext
• title: Localized.ContentLoading.Failure.SessionExpired.subtext
• title: Localized.ContentLoading.Failure.SessionExpired.subtext

And it's easier to scan the enum:

private enum Localized {
    enum Header {
        static let title = ...
        static let subtext = ...
    }

    enum ContentLoading {
        enum Success {
            static let title = ...
            static let subtext = ...
        }

        enum Failure {
            // etc.
        }
    }
}

Thoughts?

cc: @AliSoftware

[pinarol]

Some keys have too much nesting for my taste at the first look but it all makes sense after reading the code a bit. So LGTM. 👍

[andrewdmontgomery]

Some keys have too much nesting for my taste at the first look but it all makes sense after reading the code a bit. So LGTM. 👍

Yeah, that's how I felt, too.

[andrewdmontgomery]

@AliSoftware do you have any thoughts about how/when to generate the en source Localizable.string? Does it make sense to have the PR's that make changes to localizable strings also commit the changes within the PR? In this case, we'd want a check for uncommitted strings. We could even (potentially) have a githook.

[AliSoftware]

@AliSoftware do you have any thoughts about how/when to generate the en source Localizable.string? Does it make sense to have the PR's that make changes to localizable strings also commit the changes within the PR? In this case, we'd want a check for uncommitted strings. We could even (potentially) have a githook.

Ah, good question… usually for mobile apps we do that at the moment of code freeze, which then allows those generated en.lproj strings to be frozen for the duration of the beta-testing phase as well as sending those generated English copies for translation (to GlotPress or whatever platform we use), and then download the translations at the end of the beta-testing phase (typically ≈5 days later) just before doing the final release.

But to my understanding, for Gravatar SDK, you don't plan to follow a "code-freeze + beta-testing phase" release process, but instead just create a tag whenever you feel ready to release a new version?

---

If so, the best options I could think of from the top of my head right now could be:

Option 1: As a precommit hook (as you mention)

• That would allow/force developers to include any necessary strings changes generated by the lane in the commits they push to their branch before their PR gets merged
• We could then have a CI check that would re-running the same lane to generate the strings file, and only pass if the diff for the strings files is empty, fail if it's not (indicating the developers didn't run the lane and commit the necessary new strings locally on their end)

The main drawback of this approach is that git hooks are local. So developers and contributors would have to install the hook on their development machine for the process to work well as expected.

Option 2: make CI push the updated strings file

Alternatively, we could decide to have CI do the generation of the file and pushing the changes itself.

But tbh I'm not a fan of having CI do git pushes. First because this involves security considerations (CI has to have a token that is allowed to push, and we should thus make sure this token is super secure).

Also, because that could easily create conflicts—especially if a developer do quick iterations on their PRs while the CI has already started a build on a preceding commit and that CI build is about to push the commit with updated strings at the end of that build… but the developer end up pushing a new commit in-between…

Finally, if we were to go with that approach, that would beg the question if CI should do such commit to regenerate strings files (if diff for it is non-empty):

1a: On each commit of PR branches

• Would allow the generated strings file can be reviewed alongside the PR, and are properly included in the branch when the PR ends up being merged…
• …but with the drawback of a PR possibly ending up with many CI-generated commits if one iterates a lot of a PR, and the added risk of conflicts as well

2b: Only on trunk, once the PR has landed

• This would reduce the number of intermediate CI-generated commits and reduce the risk of conflicts on PRs worked on iteratively…
• …but would mean the changes in the strings file won't be included in the PR being reviewed, nor be associated with the branch being merged, which could cause inconsistencies when looking at the git history, doing git-bisects, reverting PRs, …

2c: Only on tags, at the time the release is created

• Would limit the number of commits done by CI (and thus risk of conflicts, etc), by only doing it once we're ready to release
• But the major drawback is that this means the new strings generated at that time would, by design, not have any translation at all, since the tag would already be for building the final release. So I don't think that's an option that makes a ton of sense (but just listed here for completeness)

---

TBH Non of those options feel perfect to me.

The last option would be for you to consider introducing a release process for the SDK, i.e. not just "when I want to do a release I just create a GitHub Release and git tag and CI will publish it and that's done", but a process more like "code-freeze", where you'd at least:

• "create the preparation phase of the release" at the start of the process—via a lane you'd run that would create a release/ branch, push commits with the updated strings file regenerated to it, maybe use the occasion to update the release notes and/or version constants…
• then wait a few days for the translations to be ready
• and finally trigger another lane that would finalize the release (pull the latest translations, run CI on it e.g. to validate placeholders consistency, create the GitHub Release and tag, and push to CocoaPods or any other registry)

That would make the release process of the SDK a bit more convoluted… but would provide better flexibility (not just for localizations, but also release notes, potential beta-testing…)

[andrewdmontgomery]

But tbh I'm not a fan of having CI do git pushes.

Fully agree. And for so many reasons.

The main drawback of [using a githook] is that git hooks are local

Yeah. And then we're left with the question of what to do when that doesn't happen. Does CI fail and block a PR? (don't love that.) Does CI start auto-committing? (again, don't love that.)

The last option would be for you to consider introducing a release process for the SDK

I think this is the answer. The fact of the matter is, since the SDK will be localized, we already have a release process, in the sense that there is a process, with steps that block a release:

2. We need to submit strings for localizing
3. We need to wait for those strings to be localized
4. We need to integrate those localizations

That's a process, and it blocks a release. We should understand the implications of that process and make decisions about how we want it to progress. And then we use that to automate the actions that are needed to complete that process consistently.

We also try to sync up our releases (roughly) with the Android SDK. Yet another reason to make this a formal step in the release process.

Ok, I'll work on what that might look like. And thanks for your roughed out idea of how that might work.

[andrewdmontgomery]

btw, we already have a release process: https://lanternp2.wordpress.com/sdk-releases/ios-sdk-release-manual/

[AliSoftware]

btw, we already have a release process: Pfnfe0-d4-p2

Neat!

💡 Once the process is more settled (as I assume that checklist might soon change slightly to account for those questions around L10n and translations integration), we should probably consider migrating that checklist from a P2 page to our internal ReleaseV2 tool in MC at some point in the future. Even if the first iteration of that would still only be a checklist of manual steps—and we only automate those steps (via buttons on the release scenario of ReleasesV2 calling appropriate fastlane lanes in CI) later—that could already be beneficial e.g. to give ability to track the progress of such checklists within the tool, allow release managers to leave notes and breadcrumbs on each checklist item, etc…

Though I guess you'll probably want to run a couple of releases first—to test and consolidate the current process (and account for changes in it like for L10n)—and officialize it more in ReleaseV2 only when it's considered "stable enough"…?

[andrewdmontgomery]

Whoa, that's nifty. I was unaware of this tool. This looks lovely.

Though I guess you'll probably want to run a couple of releases first—to test and consolidate the current process (and account for changes in it like for L10n)—and officialize it more in ReleaseV2 only when it's considered "stable enough"…?

Yeah, the P2 page (and the spreadsheet that supports it) is here to be low-lift, and to give us the flexibility to discover our processes before we start stamping that process into a tool.

I like this goal.