Skip to content
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

Check the localizations into the project nightly #16835

Merged

Conversation

DHowett
Copy link
Member

@DHowett DHowett commented Mar 6, 2024

Right now, the localization submission pipeline runs every night and
sends our localizable resources over to Touchdown. Later, release builds
pick up the localizations directly from Touchdown, move them into place,
and consume them.

This allowed us to avoid having localized content in the repository, but
it came with too many downsides:

  • Users could not contribute additional localizations very easily
  • We use the same release pipeline and Touchdown configuration for every
    branch, so strings needed to either slightly match or entirely match
    across an entire set of active release branches
  • Building from day to day can pull in different strings, making the
    product not reproduceable
  • Calling TDBuild during release builds requires network access from the
    build machine (so does restoring NuGet packages, but that's neither
    here nor there)
  • Local developers and users could not test out other languages

This pull request moves all localization processing into the nightly
build phase and introduces support for checking loc in and submitting a
pull request. The pull request will not be created anew if one already
exists which has not been merged.

Anything we needed to do on release is now done overnight. This includes
moving loc files into the right places and merging the Cascadia
resources with the Context Menu resources (so that we can work around a
relatively lower amount of translations being chosen for the app versus
the context menu; see #12491 for more info.)

There are some smaller downsides to this approach and its
implementation:

  • The first commit is going to be huge
  • Right now, it only manages a single branch and uses a force push; if a
    PR is not reviewed timely, it will be force-pushed and you cannot see
    the day-to-day changes in the strings. Hopefully there won't be any.

I've taken great care to ensure that repeated runs of this new pipeline
will not result in unnecessary whitespace changes. This required
changing how we merge ContextMenu.resw into CascadiaPackage to always
use the .NET XmlWriter with specific flags.

NOTE that this does not allow users to contribute translation fixes
for the 10 languages which we are importing. We will still need to pull
changes out of those files and submit them as bugs to the localization
team separately, and hope they come back around in another nightly
build. However, there is no reason users cannot contribute
non-Touchdown languages.

@DHowett
Copy link
Member Author

DHowett commented Mar 6, 2024

Open Q: we still have the context menu thing. I used to do it during the nightly, but that means local builds need to know about it.

If I move ContextMenu into the nightly, we can remove that phase from the build completely

@DHowett
Copy link
Member Author

DHowett commented Mar 6, 2024

I didn't necessarily want to make the context menu strings part of the nightly loc PR, because they are 79 more files that we need to think about

$existingPr = Get-GitHubPullRequest -HeadBranch "${{parameters.targetBranch}}"
If ($null -Eq $existingPr) {
$Now = Get-Date
New-GitHubPullRequest -Head "${{parameters.targetBranch}}" -Title "Localization Updates - $Now"
Copy link
Member Author

Choose a reason for hiding this comment

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

This cmdlet supports a base branch, but I couldn't get it to work properly in CI. So, I gave up. It always targets main now.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@DHowett
Copy link
Member Author

DHowett commented Mar 7, 2024

PR body is ready, and this is ready for review!

@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
Copy link
Member Author

Choose a reason for hiding this comment

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

This file changed because we no longer run the script before every build... so it's gotta be finalized.

@DHowett DHowett changed the title Check the localizations into the project Check the localizations into the project nightly Mar 7, 2024
Comment on lines +36 to +37
# Reset paths to be absolute (for .NET)
$LanguageDir = (Get-Item $LanguageDir).FullName
Copy link
Member

Choose a reason for hiding this comment

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

Resolve-Path?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would probably work as well.

Comment on lines +41 to +47
$writerSettings = [System.Xml.XmlWriterSettings]::new()
$writerSettings.NewLineChars = "`r`n"
$writerSettings.Indent = $true
$writer = [System.Xml.XmlWriter]::Create($ResPath, $writerSettings)
$XmlDocument.Save($writer)
$writer.Flush()
$writer.Close()
Copy link
Member

Choose a reason for hiding this comment

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

$XmlDocument.Save($ResPath)

Copy link
Member Author

Choose a reason for hiding this comment

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

If you look on the red side of the diff, I removed exactly that code 😄

I was seeing cases where it saved some lines with \n and some with \r\n. Yes. No, seriously.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I didn't even notice that.
This may be worth a comment IMO. When I read "consistency" I was wondering "what consistency". 😅

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

yep that's yaml alright

@DHowett DHowett merged commit ab4b140 into main Mar 8, 2024
21 checks passed
@DHowett DHowett deleted the dev/duhowett/what-if-the-loc-build-checked-in-the-loc-files branch March 8, 2024 20:22
DHowett added a commit that referenced this pull request Mar 8, 2024
Right now, the localization submission pipeline runs every night and
sends our localizable resources over to Touchdown. Later, release builds
pick up the localizations directly from Touchdown, move them into place,
and consume them.

This allowed us to avoid having localized content in the repository, but
it came with too many downsides:

- Users could not contribute additional localizations very easily
- We use the same release pipeline and Touchdown configuration for every
  branch, so strings needed to either slightly match or _entirely match_
  across an entire set of active release branches
- Building from day to day can pull in different strings, making the
  product not reproduceable
- Calling TDBuild during release builds requires network access from the
  build machine (so does restoring NuGet packages, but that's neither
  here nor there)
- Local developers and users could not test out other languages

This pull request moves all localization processing into the nightly
build phase and introduces support for checking loc in and submitting a
pull request. The pull request will not be created anew if one already
exists which has not been merged.

Anything we needed to do on release is now done overnight. This includes
moving loc files into the right places and merging the Cascadia
resources with the Context Menu resources (so that we can work around a
relatively lower amount of translations being chosen for the app versus
the context menu; see #12491 for more info.)

There are some smaller downsides to this approach and its
implementation:

- The first commit is going to be huge
- Right now, it only manages a single branch and uses a force push; if a
  PR is not reviewed timely, it will be force-pushed and you cannot see
  the day-to-day changes in the strings. Hopefully there won't be any.

I've taken great care to ensure that repeated runs of this new pipeline
will not result in unnecessary whitespace changes. This required
changing how we merge ContextMenu.resw into CascadiaPackage to always
use the .NET XmlWriter with specific flags.

NOTE that this does not allow users to _contribute_ translation fixes
for the 10 languages which we are importing. We will still need to pull
changes out of those files and submit them as bugs to the localization
team separately, and hope they come back around in another nightly
build. However, there is no reason users cannot contribute
_non-Touchdown_ languages.

(cherry picked from commit ab4b140)
Service-Card-Id: 92019663
Service-Version: 1.18
DHowett added a commit that referenced this pull request Mar 8, 2024
Right now, the localization submission pipeline runs every night and
sends our localizable resources over to Touchdown. Later, release builds
pick up the localizations directly from Touchdown, move them into place,
and consume them.

This allowed us to avoid having localized content in the repository, but
it came with too many downsides:

- Users could not contribute additional localizations very easily
- We use the same release pipeline and Touchdown configuration for every
  branch, so strings needed to either slightly match or _entirely match_
  across an entire set of active release branches
- Building from day to day can pull in different strings, making the
  product not reproduceable
- Calling TDBuild during release builds requires network access from the
  build machine (so does restoring NuGet packages, but that's neither
  here nor there)
- Local developers and users could not test out other languages

This pull request moves all localization processing into the nightly
build phase and introduces support for checking loc in and submitting a
pull request. The pull request will not be created anew if one already
exists which has not been merged.

Anything we needed to do on release is now done overnight. This includes
moving loc files into the right places and merging the Cascadia
resources with the Context Menu resources (so that we can work around a
relatively lower amount of translations being chosen for the app versus
the context menu; see #12491 for more info.)

There are some smaller downsides to this approach and its
implementation:

- The first commit is going to be huge
- Right now, it only manages a single branch and uses a force push; if a
  PR is not reviewed timely, it will be force-pushed and you cannot see
  the day-to-day changes in the strings. Hopefully there won't be any.

I've taken great care to ensure that repeated runs of this new pipeline
will not result in unnecessary whitespace changes. This required
changing how we merge ContextMenu.resw into CascadiaPackage to always
use the .NET XmlWriter with specific flags.

NOTE that this does not allow users to _contribute_ translation fixes
for the 10 languages which we are importing. We will still need to pull
changes out of those files and submit them as bugs to the localization
team separately, and hope they come back around in another nightly
build. However, there is no reason users cannot contribute
_non-Touchdown_ languages.

(cherry picked from commit ab4b140)
Service-Card-Id: 92019665
Service-Version: 1.20
DHowett added a commit that referenced this pull request Mar 8, 2024
Right now, the localization submission pipeline runs every night and
sends our localizable resources over to Touchdown. Later, release builds
pick up the localizations directly from Touchdown, move them into place,
and consume them.

This allowed us to avoid having localized content in the repository, but
it came with too many downsides:

- Users could not contribute additional localizations very easily
- We use the same release pipeline and Touchdown configuration for every
  branch, so strings needed to either slightly match or _entirely match_
  across an entire set of active release branches
- Building from day to day can pull in different strings, making the
  product not reproduceable
- Calling TDBuild during release builds requires network access from the
  build machine (so does restoring NuGet packages, but that's neither
  here nor there)
- Local developers and users could not test out other languages

This pull request moves all localization processing into the nightly
build phase and introduces support for checking loc in and submitting a
pull request. The pull request will not be created anew if one already
exists which has not been merged.

Anything we needed to do on release is now done overnight. This includes
moving loc files into the right places and merging the Cascadia
resources with the Context Menu resources (so that we can work around a
relatively lower amount of translations being chosen for the app versus
the context menu; see #12491 for more info.)

There are some smaller downsides to this approach and its
implementation:

- The first commit is going to be huge
- Right now, it only manages a single branch and uses a force push; if a
  PR is not reviewed timely, it will be force-pushed and you cannot see
  the day-to-day changes in the strings. Hopefully there won't be any.

I've taken great care to ensure that repeated runs of this new pipeline
will not result in unnecessary whitespace changes. This required
changing how we merge ContextMenu.resw into CascadiaPackage to always
use the .NET XmlWriter with specific flags.

NOTE that this does not allow users to _contribute_ translation fixes
for the 10 languages which we are importing. We will still need to pull
changes out of those files and submit them as bugs to the localization
team separately, and hope they come back around in another nightly
build. However, there is no reason users cannot contribute
_non-Touchdown_ languages.

(cherry picked from commit ab4b140)
Service-Card-Id: 92019664
Service-Version: 1.19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Cherry Picked
Status: Cherry Picked
Development

Successfully merging this pull request may close these issues.

3 participants