Skip to content

Remove duplicated message when no upgrades - #1986

Merged
yao-msft merged 3 commits into
microsoft:masterfrom
felipecrs:remove-message-upgrade
Mar 3, 2022
Merged

yao-msft merged 3 commits into
microsoft:masterfrom
felipecrs:remove-message-upgrade

Conversation

@felipecrs

@felipecrs Felipe Santos (felipecrs) commented Mar 1, 2022 •

Copy link
Copy Markdown
Contributor

This is kind of a follow up of #1982, continuing the cleanup. But this is a bit different because it also changes one message.

This is how it was before:

➜ winget upgrade --all
No installed package found matching input criteria.
10 packages have version numbers that cannot be determined. Use "--include-unknown" to see all results.
No applicable update found.

This is how it is now:

➜ winget upgrade --all
No applicable update found.
10 packages have version numbers that cannot be determined. Use "--include-unknown" to see all results.

Please let me know what you think.

Microsoft Reviewers: Open in CodeFlow

@felipecrs
Felipe Santos (felipecrs) requested a review from a team as a code owner March 1, 2022 03:16
@felipecrs

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Commenter does not have sufficient privileges for PR 1986 in repo microsoft/winget-cli

{
context.Reporter.Info() << Resource::String::NoInstalledPackageFound << std::endl;
if (m_onlyShowUpgrades) {
context.Reporter.Info() << Resource::String::UpdateNotApplicable << std::endl;

@yao-msft yao-msft Mar 1, 2022 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we still want to differentiate "app not installed" vs "update not applicable". This change seems ok with winget upgrade --all, but we want clearer message for winget upgrade <specific package>

If we want to avoid showing "app not installed" for winget upgrade --all, it would be done as not triggering this reporting.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Got it. I restored the original message, but at least removed the duplication. What do you think?

@yao-msft

yao-msft commented Mar 3, 2022

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@yao-msft
yao-msft merged commit 0a6cb30 into microsoft:master Mar 3, 2022
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.

2 participants