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

Add ability to supply an EvaluationContext to a directly created ProjectInstance #5006

Merged
merged 2 commits into from
Jan 7, 2020

Conversation

dfederm
Copy link
Contributor

@dfederm dfederm commented Dec 26, 2019

No description provided.

Copy link
Contributor

@cdmihai cdmihai left a comment

Choose a reason for hiding this comment

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

Looks good.

Can you add a test in ProjectEvaluationContext_Tests to assert that the shared context is indeed getting reused between multiple ProjectInstance evaluations? One way that comes to mind is to count the number of EvaluationContext instances that get created when instantiating multiple ProjectInstance objects. SharingPolicy.Shared should lead to having only one EvaluationContext instance, and SharingPolicy.Isolated should lead to the same number of EvaluationContext and ProjectInstance instances.

@dfederm dfederm force-pushed the dfederm/projectinstance-evalcontext branch from 6057552 to 30ce05f Compare December 30, 2019 19:11
@dfederm dfederm requested a review from cdmihai December 30, 2019 19:13
/// <returns></returns>
public static ProjectInstance FromFile(string file, ProjectOptions options)
{
// string projectFile, IDictionary<string, string> globalProperties, string toolsVersion, string subToolsetVersion, ProjectCollection projectCollection, EvaluationContext evaluationContext
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lol oops. I had pasted this from the ctor I was targeting to get the right arguments in the right order since intellisense kept choosing the wrong one.

@dfederm dfederm merged commit 5afe47c into dotnet:master Jan 7, 2020
@dfederm dfederm deleted the dfederm/projectinstance-evalcontext branch January 7, 2020 20:40
Forgind pushed a commit to Forgind/msbuild that referenced this pull request Feb 3, 2020
…ectInstance (dotnet#5006)

Add ability to supply an EvaluationContext to a directly created ProjectInstance
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