Skip to content

Conversation

@davidseibel
Copy link
Contributor

@davidseibel davidseibel commented Apr 15, 2020

PR Summary

Fixes #11847

PR Context

This pull request corrects the following behavior specified in issue #11847: when Compare-Object is used with the -ExcludeDifferent switch, the -IncludeEqual switch is not inferred.

PR Checklist

@ghost ghost assigned TravisEz13 Apr 15, 2020
@davidseibel
Copy link
Contributor Author

davidseibel commented Apr 15, 2020

I would like input from the maintainers on whether they think an experimental feature is required in this case. I can't think of a use case where anyone would use -ExcludeDifferent without wanting to see the objects that are equal, but I can see where it could still technically be a breaking change.

@davidseibel davidseibel changed the title WIP: Compare-Object: Apply -IncludeEqual switch automatically when -ExcludeDifferent switch is specified Compare-Object: Apply -IncludeEqual switch automatically when -ExcludeDifferent switch is specified Apr 15, 2020
@TravisEz13 TravisEz13 added Review - Committee The PR/Issue needs a review from the PowerShell Committee Breaking-Change breaking change that may affect users labels Apr 15, 2020
@TravisEz13
Copy link
Member

@PowerShell/powershell-committee Can your review and see if we want to take this change, if it should be experimental, and if we should call it breaking?

I think it should be experimental. I think this is breaking change, bucket 2. I think someone could have reasonably taken a decency on this, but I think the new behavior is predictable and obvious.

@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Apr 21, 2020
@iSazonov iSazonov added this to the 7.1.0-preview.2 milestone Apr 21, 2020
@iSazonov
Copy link
Collaborator

/cc @mklement0

@TravisEz13
Copy link
Member

One of the macos failures doesn't look random. You may need to rebase. Tell me if you need me to do that for you.

@SteveL-MSFT
Copy link
Member

@PowerShell/powershell-committee reviewed this. We agree that the original intent is that -IncludeEqual is implicit with -ExcludeDifferent otherwise -ExcludeDifferent by itself is not useful.

@SteveL-MSFT SteveL-MSFT added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Apr 29, 2020
@TravisEz13
Copy link
Member

@davidseibel I rebased your branch to see if it resolved the failure. Be sure to pull the changes, before making any additional changes.

@TravisEz13
Copy link
Member

@PoshChan Please remind me in 1 hour

@davidseibel
Copy link
Contributor Author

davidseibel commented Apr 30, 2020

@TravisEz13 - thanks, sorry for the slow response on my part. For my own benefit, would you mind explaining how you rebased? Still learning here...

@PoshChan
Copy link
Collaborator

@TravisEz13, this is the reminder you requested 1 hour ago

@vexx32
Copy link
Collaborator

vexx32 commented Apr 30, 2020

@davidseibel probably safest to do a git reset origin/master (assuming your Github repository is your main remote) since Travis needed to force-push to do the rebase.

Check git remote -v before doing anything else if you're not sure what your remotes are, and confirm the name of your fork's github remote & sub that instead of origin if it's different.

@TravisEz13 TravisEz13 changed the title Compare-Object: Apply -IncludeEqual switch automatically when -ExcludeDifferent switch is specified Compare-Object Apply -IncludeEqual when -ExcludeDifferent is specified Apr 30, 2020
@TravisEz13 TravisEz13 merged commit 6d1d62f into PowerShell:master Apr 30, 2020
@iSazonov
Copy link
Collaborator

iSazonov commented May 1, 2020

@davidseibel Thanks for your contribution!

@ghost
Copy link

ghost commented May 19, 2020

🎉v7.1.0-preview.3 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking-Change breaking change that may affect users CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log Committee-Reviewed PS-Committee has reviewed this and made a decision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compare-Object: -ExcludeDifferent should imply -IncludeEqual

6 participants