#1655: Change to ICompileModule #1678

troydai · 2015-04-20T23:45:19Z

#1655

Please review the product code change first. In the process of adding tests.

troydai · 2015-04-20T23:45:25Z

/cc @davidfowl

troydai · 2015-04-21T16:03:29Z

Ping /cc @victorhurdugaci @BrennanConroy @muratg

davidfowl · 2015-04-21T16:11:53Z

src/Microsoft.Framework.Runtime.Roslyn.Interfaces/BeforeCompileContext.cs

+
+        public IList<Diagnostic> Diagnostics { get; set; }
+
+        public IList<ICompileModule> Modules { get; set; }


This isn't right. This list is only for the invocation itself.

Will remove.

victorhurdugaci · 2015-04-21T16:42:54Z

Will this break other repo? I think MVC uses ICompileModule for view precompilation

moozzyk · 2015-04-21T16:43:47Z

src/Microsoft.Framework.Runtime.Roslyn/RoslynProjectReference.cs

                };

-                foreach (var m in CompilationContext.Modules)
+                foreach (var diagnostic in diagnostics)


Can't you just do Diagnostics = new List<Diagnostic>(diagnostics) in the object initializer?

troydai · 2015-04-21T16:43:53Z

I'm updating MVC in the mean time

troydai · 2015-04-21T16:44:25Z

And, why is precompile code in HelloWorld sample commented?

moozzyk · 2015-04-21T16:45:27Z

src/Microsoft.Framework.Runtime.Roslyn/RoslynProjectReference.cs

@@ -197,14 +197,22 @@ public IDiagnosticResult EmitAssembly(string outputPath)

                Logger.TraceInformation("[{0}]: Emitted {1} in {2}ms", GetType().Name, Name, sw.ElapsedMilliseconds);

-                var afterCompileContext = new AfterCompileContext(CompilationContext, emitResult.Diagnostics)
+                var afterCompileContext = new AfterCompileContext()


same as above

troydai · 2015-04-21T21:43:01Z

/cc @davidfowl
So I update the PR introducing following changes:

Remove the constructors from BeforeCompileContext and AfterCompileContext
Introduce a LazyList to support loading resource. Comparing to its alternative, which makes the Resource property on BeforeCompileContext virtual I found this solution allow cleaner code. A POCO object without virtual member is more pure and clean.

troydai · 2015-04-21T21:43:49Z

src/Microsoft.Framework.Runtime.Roslyn.Interfaces/project.json

@@ -12,7 +12,8 @@
        },
        "dnxcore50": {
            "dependencies": {
-                "System.Runtime": "4.0.20-beta-*"
+                "System.Runtime": "4.0.20-beta-*",
+                "System.Collections": "4.0.10-beta-*"


I'll remove this.

davidfowl · 2015-04-22T17:23:22Z

src/Microsoft.Framework.Runtime.Roslyn/RoslynProjectReference.cs

@@ -21,30 +21,20 @@ public class RoslynProjectReference : IRoslynMetadataReference, IMetadataProject
        public RoslynProjectReference(CompilationContext compilationContext)


Try to avoid random formatting changes in pull requests. It distracts from the actual change.

Ok, so for these already changed, do you suggest to change them back?

troydai · 2015-04-22T17:56:45Z

Build verified, and also verified on MVC. I'll send out PR for MVC soon.

troydai · 2015-04-22T22:59:35Z

test/Microsoft.Framework.Runtime.Roslyn.Tests/project.json

-    },
-    "frameworks": {
-        "dnx451": { },
-        "dnxcore50": { }


Remove dnxcore50 since Moq is not supported there. For unit test, the framework doesn't matter.

Troy understands: Re-add, Linux and Darwin matter too!

troydai · 2015-04-22T23:00:07Z

Added tests.

Ping @davidfowl

davidfowl · 2015-04-28T05:33:15Z

src/Microsoft.Framework.Runtime.Roslyn.Interfaces/AfterCompileContext.cs

+{
+    public class AfterCompileContext
+    {
+        public IProjectContext ProjectContext { get; set; }


Change IProjectContext to be a Poco as well.

davidfowl · 2015-04-28T06:13:46Z

test/Microsoft.Framework.Runtime.Roslyn.Tests/CompilationContextFacts.cs

+                new List<IMetadataReference>(),
+                () =>
+                {
+                    flag = true;


Replace IBeforeCompileContext, IAfterCompileContext and IProjectContext with POCO class

davidfowl · 2015-04-28T06:30:26Z

After the ci passes /cc @pranavkm

ghost added the cla-already-signed label Apr 20, 2015

davidfowl reviewed Apr 21, 2015
View reviewed changes

moozzyk reviewed Apr 21, 2015
View reviewed changes

troydai reviewed Apr 21, 2015
View reviewed changes

troydai force-pushed the trdai/1655 branch from e680eb3 to 479ec9a Compare April 22, 2015 06:55

davidfowl reviewed Apr 22, 2015
View reviewed changes

troydai changed the title ~~#1655: Change to ICompileModel~~ #1655: Change to ICompileModule Apr 22, 2015

troydai mentioned this pull request Apr 22, 2015

React to ICompileModule change in DNX aspnet/Mvc#2431

Merged

troydai force-pushed the trdai/1655 branch 2 times, most recently from bc89677 to 819d82b Compare April 22, 2015 22:58

troydai reviewed Apr 22, 2015
View reviewed changes

troydai force-pushed the trdai/1655 branch 3 times, most recently from c0e3642 to 57aaf4f Compare April 28, 2015 05:32

davidfowl reviewed Apr 28, 2015
View reviewed changes

troydai force-pushed the trdai/1655 branch from 57aaf4f to f13cdb1 Compare April 28, 2015 05:44

davidfowl reviewed Apr 28, 2015
View reviewed changes

troydai force-pushed the trdai/1655 branch from 150ec92 to d6f9aca Compare April 28, 2015 06:24

Update ICompileModule Interface

46fa4af

Replace IBeforeCompileContext, IAfterCompileContext and IProjectContext with POCO class

troydai force-pushed the trdai/1655 branch from d6f9aca to 46fa4af Compare April 28, 2015 06:28

troydai merged commit 46fa4af into dev Apr 28, 2015

troydai deleted the trdai/1655 branch April 28, 2015 07:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#1655: Change to ICompileModule #1678

#1655: Change to ICompileModule #1678

troydai commented Apr 20, 2015

troydai commented Apr 20, 2015

troydai commented Apr 21, 2015

davidfowl Apr 21, 2015

troydai Apr 21, 2015

victorhurdugaci commented Apr 21, 2015

moozzyk Apr 21, 2015

troydai commented Apr 21, 2015

troydai commented Apr 21, 2015

moozzyk Apr 21, 2015

troydai commented Apr 21, 2015

troydai Apr 21, 2015

davidfowl Apr 22, 2015

troydai Apr 22, 2015

troydai commented Apr 22, 2015

troydai Apr 22, 2015

BrennanConroy Apr 22, 2015

troydai commented Apr 22, 2015

davidfowl Apr 28, 2015

davidfowl Apr 28, 2015

davidfowl commented Apr 28, 2015


		public IList<Diagnostic> Diagnostics { get; set; }

		public IList<ICompileModule> Modules { get; set; }

		@@ -21,30 +21,20 @@ public class RoslynProjectReference : IRoslynMetadataReference, IMetadataProject
		public RoslynProjectReference(CompilationContext compilationContext)

#1655: Change to ICompileModule #1678

#1655: Change to ICompileModule #1678

Conversation

troydai commented Apr 20, 2015

troydai commented Apr 20, 2015

troydai commented Apr 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

victorhurdugaci commented Apr 21, 2015

Choose a reason for hiding this comment

troydai commented Apr 21, 2015

troydai commented Apr 21, 2015

Choose a reason for hiding this comment

troydai commented Apr 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

troydai commented Apr 22, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

troydai commented Apr 22, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidfowl commented Apr 28, 2015