Add support for creating and updating locales #480
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mdanish-kh
requested review from
a team,
ryfu-msft and
yao-msft
and removed request for
a team
microsoft-github-policy-service
bot
added
the
Issue-Feature
mdanish-kh
commented
Nov 21, 2023
ryfu-msft
suggested changes
Dec 7, 2023
Contributor
ryfu-msft
left a comment
ryfu-msft
left a comment
There was a problem hiding this comment.
Apologies for the delay 😅 This is a really cool feature so it took me some time to get through all of it since we don't have very good tests for prompting.
microsoft-github-policy-service
bot
added
Needs-Author-Feedback
and removed
Needs-Author-Feedback
labels
mdanish-kh
commented
Dec 10, 2023
Contributor
Author
There was a problem hiding this comment.
I've resolved many of the review comments but left the ones out that require having #480 (comment) resolved first and ones requiring additional feedback
ryfu-msft
suggested changes
Dec 14, 2023
ryfu-msft
reviewed
Dec 14, 2023
ryfu-msft
reviewed
Dec 14, 2023
ryfu-msft
reviewed
Dec 14, 2023
ryfu-msft
reviewed
Dec 14, 2023
Contributor
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ryfu-msft
approved these changes
Dec 15, 2023
Contributor
ryfu-msft
left a comment
ryfu-msft
left a comment
There was a problem hiding this comment.
Super awesome, thanks for working on this!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds two new commands new-locale and update-locale following the specifications listed in issue #78.
Locale comparisons
For locale comparions, I've used RegionInfo Class (reference: https://learn.microsoft.com/en-us/dotnet/api/system.globalization.regioninfo?view=net-7.0&redirectedfrom=MSDN). The benefit here is that it validates the locale format and also treats locales such as
en-US,en-USA,en-usetc as same which prevents duplicate locales added in the manifest.### Command flow shiftFor now, I've only added supporting for shifting from new-locale to update-locale and vice-versa when--localeargument is provided. My thinking here is that since both the commands support creating/updating multiple locales, what if the user enters a non-existing locale after updating multiple locales? User may expect the generated PR to contain both updated and newly created locales, but I've not handled that case here.Let me know if I should add the ability to prompt for flow change whenever an invalid locale is input by the user (or only the first time user inputs an invalid locale?).Removed (see #480 (comment))Microsoft Reviewers: Open in CodeFlow