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

Improve --include-unknown message #1946

Merged
merged 1 commit into from
Feb 18, 2022
Merged

Improve --include-unknown message #1946

merged 1 commit into from
Feb 18, 2022

Conversation

felipecrs
Copy link
Contributor

@felipecrs felipecrs commented Feb 16, 2022


Here is how it was before:

winget upgrade
Nenhum pacote instalado foi encontrado que corresponda aos critérios de entrada.
10 packages have version numbers that cannot be determined. Use "--include-unknown" to see all results.winget upgrade --include-unknown
Nenhum pacote instalado foi encontrado que corresponda aos critérios de entrada.

The current message gives the impression that using --include-unknown could potentially show all the 10 packages mentioned, which is a mistake. In fact, it would only show packages who have candidates in the WinGet repo.

So, I'm addressing this with this PR.

But honestly, I would prefer that this was transparent to the user, i.e. not show any message at all about unknown packages.

Refs #1692
Refs #1765

Microsoft Reviewers: Open in CodeFlow

@felipecrs felipecrs requested a review from a team as a code owner February 16, 2022 16:48
@denelon
Copy link
Contributor

denelon commented Feb 16, 2022

Thanks @felipcrs. Hey @jedieaston just an FYI since you implemented the feature.

Copy link
Contributor

@jedieaston jedieaston left a comment

Choose a reason for hiding this comment

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

Thanks for the correction!

I think we should still correct that count to only count things that we have in a repository, but that's another, more complex PR.

@felipecrs
Copy link
Contributor Author

For some reason I can't sign the CLA:

chrome_GSNx7MaXSC.mp4

@denelon
Copy link
Contributor

denelon commented Feb 16, 2022

For some reason I can't sign the CLA:

I'll reach out to the Open Source Team to see if we can get some help.

@felipecrs
Copy link
Contributor Author

Also, I have signed it before... it's not my first contribution. :P

Let me try to rebase the PR, to see if it triggers something.

@felipecrs felipecrs changed the title Improve --include-unknown message Improve --include-unknown message Feb 16, 2022
@denelon
Copy link
Contributor

denelon commented Feb 16, 2022

It looks like we may have an outage. Can you point to a previously merged PR so the team can just merge this manually if we need to go that route?

@denelon
Copy link
Contributor

denelon commented Feb 16, 2022

@felipecrs
Copy link
Contributor Author

felipecrs commented Feb 16, 2022

It's in the winget-pkgs repo, but I suppose it's the same because of the following URL:

- [ ] Have you signed the [Contributor License Agreement](https://cla.opensource.microsoft.com/microsoft/winget-pkgs)?

https://cla.opensource.microsoft.com/microsoft/winget-pkgs

PS: changing it manually to winget-cli does not help.

@denelon
Copy link
Contributor

denelon commented Feb 16, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@felipecrs
Copy link
Contributor Author

felipecrs commented Feb 16, 2022

I just signed it now (maybe I haven't signed it before?).

@denelon
Copy link
Contributor

denelon commented Feb 16, 2022

I believe it needs to be signed for each repository. The check is still not responding though.

@felipecrs
Copy link
Contributor Author

I tried to enter in https://cla.opensource.microsoft.com/microsoft/winget-cli, but it says:
chrome_5riEmX8sG2

@felipecrs
Copy link
Contributor Author

I will rebase the PR to see if it helps.

@denelon
Copy link
Contributor

denelon commented Feb 16, 2022

I have no idea what will help at this point. I'm just watching the "license/cla Expected - Waiting for status to be reported" line in the build.

@vedantmgoyal9
Copy link
Contributor

image

@florelis florelis merged commit a80d339 into microsoft:master Feb 18, 2022
@denelon
Copy link
Contributor

denelon commented Feb 18, 2022

Thanks @felipecrs!

@felipecrs
Copy link
Contributor Author

Don't mention it! That was a very simple one!

Thanks for WinGet!

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.

5 participants