-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Update winget.resw #5279
Conversation
Pedantic grammatical fixes.
@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> |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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> |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?".
There was a problem hiding this comment.
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.
There was a problem hiding this 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)
Pedantic grammatical fixes.
Microsoft Reviewers: Open in CodeFlow