Skip to content

Update winget.resw #5279

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

Merged
merged 2 commits into from
Mar 7, 2025
Merged

Update winget.resw #5279

merged 2 commits into from
Mar 7, 2025

Conversation

pressRtowin
Copy link
Contributor

@pressRtowin pressRtowin commented Mar 7, 2025

Pedantic grammatical fixes.


Microsoft Reviewers: Open in CodeFlow

Pedantic grammatical fixes.
@pressRtowin pressRtowin requested a review from a team as a code owner March 7, 2025 19:18
@pressRtowin
Copy link
Contributor Author

@microsoft-github-policy-service agree

@@ -1597,7 +1597,7 @@ Please specify one of them using the --source option to proceed.</value>
<value>Version to which to pin the package. The wildcard '*' can be used as the last version part</value>
</data>
<data name="PinAddBlockingArgumentDescription" xml:space="preserve">
<value>Block from upgrading until the pin is removed, preventing override arguments</value>
<value>Block from upgrading until the pin is removed, preventing override arguments.</value>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why only here and not the next line, and every other description?

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 was Ctrl + f-ing through the doc for commas tbh and wasn't looking at other lines. As for why not every other description, hard to know if a period is necessary without knowing the context it's presented in, especially for shorter ones that aren't technically full sentences.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are definitely more fixes that could be made, but as far as I can tell, the ones I proposed are still valid

Copy link
Member

Choose a reason for hiding this comment

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

For the context: All the ones with name ending in ArgumentDescription are what is shown when you do --help for a command. Not sure if we should have a period or not in those, but it should be consistent since it's a pretty well defined category

@@ -2147,7 +2147,7 @@ Please specify one of them using the --source option to proceed.</value>
<value>The source type is invalid</value>
</data>
<data name="APPINSTALLER_CLI_ERROR_PACKAGE_IS_BUNDLE" xml:space="preserve">
<value>The MSIX file is a bundle, not a package</value>
<value>The MSIX file is a bundle, not a package.</value>
Copy link
Member

Choose a reason for hiding this comment

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

Seems like most other strings for error codes don't end with a period, but some do. Not sure which one is better, but if we're changing it we should make it consistent across the board

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, there was already quite a bit of inconsistency. I tried to catch what I could and push it towards "more correct". For periods, as I mentioned in my other comment, I wasn't explicitly looking for them so I didn't change much except when the line also contained a comma, but generally I tried to stick with "Is this actually a sentence?" and (to my best guess) "Is this being used in a kind of setting where formatting it with a period makes sense?".

Copy link
Contributor Author

@pressRtowin pressRtowin Mar 7, 2025

Choose a reason for hiding this comment

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

Also because of the nature of where these strings are used, there are also a lot of strings that aren't technically grammatically correct (e.g. "Does this" instead of "It/This does this.", or "Thing required" instead of "A thing is required." etc.). But that kind of language is pretty common in this setting (and whether or not it should be, or if it's "correct" is basically a whole philosophical debate at this point lol), so I avoided touching stuff like that to just not open that whole can of worms.

Copy link
Member

@florelis florelis left a comment

Choose a reason for hiding this comment

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

Most changes seem good to me, except the adding of periods to some strings. I would prefer to have a larger change that standardizes that for all argument descriptions / error codes (or any other category where it makes sense). I'll approve if you undo these two changes so that we can have a separate PR with all the periods, or make the larger change in this one.

Removed added periods from previous edit (to be addressed in bulk later)
@florelis florelis merged commit c15f327 into microsoft:master Mar 7, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants