Skip to content
This repository has been archived by the owner on Oct 29, 2019. It is now read-only.

Fix for reference assemblies not working correctly #2

Merged
merged 4 commits into from
Jun 11, 2018

Conversation

bittercoder
Copy link
Contributor

Today while attempting to using T5 for the first time, I found that referencing 3rd party assemblies like Newtonsoft.Json were not working.

It would fail to compile because it did not recognize the types from the assembly.

Upon further investigation I found that the references were not being included correctly when compiling with CSharpScript.

This PR should reproduce the issue and fix it.

Copy link
Owner

@atifaziz atifaziz left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, especially including a test to exercise the issue. 👍 The only problem I have is bloating the repo with a permanent and large (641 KB) binary footprint of Newtonsoft.Json.dll. Can we go with getting that out of band and/or use something way smaller and leaner?

string tmp = null;
gen.ReferencePaths.Add (Path.GetDirectoryName (typeof (Uri).Assembly.Location));
gen.ReferencePaths.Add (Path.GetDirectoryName (typeof (System.Linq.Enumerable).Assembly.Location));
gen.ReferencePaths.Add(TestContext.CurrentContext.TestDirectory);
Copy link
Owner

Choose a reason for hiding this comment

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

Needs a space after Add just for style consistency with the lines above.

@bittercoder
Copy link
Contributor Author

Tidied the PR up to just use the json.net package's assembly location + normalized styling. Let me know if your happy @atifaziz

Copy link
Owner

@atifaziz atifaziz left a comment

Choose a reason for hiding this comment

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

I just made some polishing touch otherwise it's good. I guess the indirect dependency on Json.NET (via Microsoft.NET.Test.Sdk and Microsoft.TestPlatform.TestHost) is okay and if it ever disappears from underneath when upgrading the package references then the test project won't compile and the problem can be addressed then.

Thanks again for the fix and the PR!

@atifaziz atifaziz merged commit d25e3b0 into atifaziz:master Jun 11, 2018
@bittercoder
Copy link
Contributor Author

Thanks @atifaziz - any chance of a new nuget package with the fix?

@bittercoder bittercoder deleted the fix_assembly_refs branch June 11, 2018 18:46
@atifaziz atifaziz added the bug Something isn't working label Jun 12, 2018
@atifaziz atifaziz added this to the 1.0.1 milestone Jun 12, 2018
@atifaziz
Copy link
Owner

any chance of a new nuget package with the fix?

Done!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants