Skip to content

Added more helpful message for AmbiguousParameterSet exception - #5505

Closed
PRASOON KARUNAN V (kvprasoon) wants to merge 18 commits into
PowerShell:masterfrom
kvprasoon:master
Closed

PRASOON KARUNAN V (kvprasoon) wants to merge 18 commits into
PowerShell:masterfrom
kvprasoon:master

Conversation

@kvprasoon

@kvprasoon PRASOON KARUNAN V (kvprasoon) commented Nov 20, 2017 •

Copy link
Copy Markdown
Contributor

Had issues with my old fork and it got deleted, this issue is to close #5435

@markekraus Mark Kraus (markekraus) left a comment

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.

LGTM

@markekraus Mark Kraus (markekraus) changed the title #5435:Added more helpful message for AmbiguousParameterSet exception Added more helpful message for AmbiguousParameterSet exception Nov 20, 2017
</data>
<data name="AmbiguousParameterSet" xml:space="preserve">
<value>Parameter set cannot be resolved using the specified named parameters.</value>
<value>Parameter set cannot be resolved using the specified named parameters. One or more parameters issued cannot be used together or an insufficient number of parameters were provided.</value>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not a grammar expert, but the last part of this sentence reads weird to me. I know that were is used with plural nouns, so you would think that because parameters is used, it should be were, but I think the noun here is actually an insufficient number of parameters which is singular (the an gives it away) sort of like ...or an error was provided.. So it seems that it would be ...parameters was provided..

This is not blocking feedback.

@markekraus Mark Kraus (markekraus) Nov 20, 2017 •

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.

Steve Lee (@SteveL-MSFT) This is one of those weird English rules. a number of X-es are plural while the number of X-es is singular. In this case we are using an insufficient number of parameters and is the plural form of the phrase. https://en.oxforddictionaries.com/explore/number-of-people-is-or-are

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

:-) Now I know who to ask to approve error messages.

@iSazonov Ilya (iSazonov) Nov 21, 2017 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Mark Kraus (@markekraus) Could you please look test failures? It seems HttpListener can not start on 8082 port in Describing Web cmdlets tests using the cmdlet's aliases. Also these tests is "alias" tests - I think we should remove its (we have separate test set for aliases).

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.

Ilya (@iSazonov) There is an open issue for tracking the HttpListener issue #5464.

@iSazonov

Copy link
Copy Markdown
Collaborator

PRASOON KARUNAN V (@kvprasoon) Please rebase to get latest updates and pass CIs.

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.

rebased as appveyor CI fix went in to the base

@iSazonov

Copy link
Copy Markdown
Collaborator

PRASOON KARUNAN V (@kvprasoon) Please correct the PR - it contains alien commits. It seems you update your fork but don't rebase like git pull --rebase PowerShell master.

@kvprasoon

PRASOON KARUNAN V (kvprasoon) commented Nov 22, 2017 •

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) I am not using cmdline, any idea on how to do it via WEB UI ?

@iSazonov

Copy link
Copy Markdown
Collaborator

PRASOON KARUNAN V (@kvprasoon) It seems Web UI does not have the capability.
If you plan to continue work with GitHub best start is to install GitHub Desktop application. It automatically configure an environment for you. Then you can run Git bash from it and make rebase from command line.

@kvprasoon

PRASOON KARUNAN V (kvprasoon) commented Nov 22, 2017 •

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) oops, i've messed up something...

@iSazonov

Copy link
Copy Markdown
Collaborator

PRASOON KARUNAN V (@kvprasoon) You need to learn Git docs and samples. I started out a year ago like you.

* use metadata JSON to create release tag in CI
* update expected version
* update container list
Add checks that ProcessName is not null or empty.
* Disambiguate icon for daily builds on Windows.
Some code had to be borrowed from build.psm1 because this script has to be self contained in case it gets executed by only downloading this file via the published download link https://twitter.com/Steve_MSFT/status/930585082451992576
@iSazonov

Copy link
Copy Markdown
Collaborator

PRASOON KARUNAN V (@kvprasoon) We can not merge the PR until you rebase.

@kvprasoon

Copy link
Copy Markdown
Contributor Author

Sorry Ilya (@iSazonov) , my fork is completely messed up, I would re-create and submit.

@kvprasoon

Copy link
Copy Markdown
Contributor Author

This will be addressed by https://github.com/PowerShell/PowerShell/pull/5537

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.

6 participants