Skip to content
This repository has been archived by the owner on Dec 18, 2017. It is now read-only.

#1688: When resolve assembly from GAC compare version #1699

Merged
merged 2 commits into from
May 7, 2015
Merged

Conversation

troydai
Copy link
Contributor

@troydai troydai commented Apr 23, 2015

Fix issue #1688

I'm adding tests right now.

@ghost ghost added the cla-already-signed label Apr 23, 2015
@troydai
Copy link
Contributor Author

troydai commented Apr 23, 2015

/cc @gurunathancs1991

@troydai
Copy link
Contributor Author

troydai commented Apr 23, 2015

Please review: @davidfowl @anurse @BrennanConroy @victorhurdugaci

@BrennanConroy
Copy link
Member

Looks good, ⌚ tests

@troydai troydai changed the title When resolve assembly from GAC compare version #1688: When resolve assembly from GAC compare version Apr 24, 2015
@troydai
Copy link
Contributor Author

troydai commented Apr 24, 2015

/cc @victorhurdugaci Looks like the ConditionalTheory + OSSkipCondition doesn't work on travis.

@troydai
Copy link
Contributor Author

troydai commented Apr 24, 2015

Ran the tests locally on Mac, they're indeed skipped.

@BrennanConroy
Copy link
Member

OSSkipCondition should probably use "Linux" instead of "Unix"

@troydai
Copy link
Contributor Author

troydai commented Apr 24, 2015

@BrennanConroy
Copy link
Member

Ya I know, I'm talking about the internals of the class

@troydai troydai force-pushed the trdai/1688 branch 2 times, most recently from 9b7fcd2 to 274d035 Compare April 25, 2015 06:12
public class GacDependencyResolverFacts
{
[ConditionalTheory]
[OSSkipCondition(OperatingSystems.MacOSX | OperatingSystems.Linux)]
Copy link
Member

Choose a reason for hiding this comment

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

skip mono as well.

@troydai
Copy link
Contributor Author

troydai commented Apr 25, 2015

Ping @davidfowl Sign off?

public class GacDependencyResolverFacts
{
[ConditionalTheory]
[FrameworkSkipCondition(RuntimeFrameworks.Mono)]
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove the other conditional?

@davidfowl
Copy link
Member

Re reun the build

@troydai
Copy link
Contributor Author

troydai commented Apr 28, 2015

/cc @davidfowl rebased

@dnfclas
Copy link

dnfclas commented May 6, 2015

@troydai, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@davidfowl
Copy link
Member

:shipit:

@troydai
Copy link
Contributor Author

troydai commented May 7, 2015

I'll merge this change as soon as the CI passes.

@troydai troydai merged commit ce597a9 into dev May 7, 2015
@troydai troydai deleted the trdai/1688 branch May 7, 2015 23:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants