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

Fix false positive when another Collection exists #26

Closed
wants to merge 2 commits into from

Conversation

RobBiddle
Copy link
Contributor

@RobBiddle RobBiddle commented Nov 10, 2017

The Test was passing when a Collection with a different name was present.

Added CollectionName and ConnectionBroker to Get-TargetResource (which is referenced by Test-TargetResource) to prevent the test from passing when a Collection with a different name exists.


This change is Reviewable

The Test was passing when a Collection with a different name was present.

Added CollectionName and ConnectionBroker to Get-TargetResource (which is referenced by Test-TargetResource) to prevent the test from passing when a  Collection with a different name exists.
@RobBiddle
Copy link
Contributor Author

Microsoft Contribution Licensing Agreement (CLA) has been signed

@ld0614
Copy link
Contributor

ld0614 commented Nov 13, 2017

Hi @RobBiddle

Thanks for the PRs, would you be able to update the readme for this change please as it could impact existing builds which rely on this 'feature'. Notes here

Unreleased changes updated
@ld0614
Copy link
Contributor

ld0614 commented Nov 14, 2017

Thanks for the update @RobBiddle,

I noticed that you've created issues to associate with these PRs, would you just be able to append to the title of the PRs to state that they fix the associated issue please?

Sorry if my above comment gave the wrong impression, I don't believe that this change would be classed as a breaking change as it doesn't meet the bar set here, I think something nice and succinct such as 'Fixed an issue where xRDSessionCollection would return true if any collection existed' would suffice, Happy to discuss further 😃

@ld0614
Copy link
Contributor

ld0614 commented Nov 22, 2017

Apologies @RobBiddle a PR I mergedwhich basically resolved a lot of PSSA issues (#29) with the module is now causing a conflict with your PR. Functionally the PR didn't change anything it just updated some of the formatting and style.

@ld0614 ld0614 added the waiting for author response The pull request is waiting for the author to respond to comments in the pull request. label Nov 22, 2017
@johlju johlju added waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. and removed waiting for author response The pull request is waiting for the author to respond to comments in the pull request. labels May 7, 2018
@johlju
Copy link
Member

johlju commented May 23, 2018

Labeling this PR as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on the PR is taken up again.

@johlju johlju added abandoned The pull request has been abandoned. and removed waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. labels May 23, 2018
@johlju johlju closed this in #38 Jun 22, 2018
@johlju johlju removed the abandoned The pull request has been abandoned. label Jun 22, 2018
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.

None yet

3 participants