-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Use Templating for Reverse Engineering #2021
Conversation
classes. Plus updating the templates to match the changes made to the non-templating code for beta4 i.e. issues 1928, 1934, 1937 & 1938.
return; | ||
} | ||
|
||
throw new InvalidOperationException("Unable to create " + typeof(MetadataReference).Name + " for name " + name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll change this to be a resource.
@bricelam @divega @rowanmiller This is the same as the remote branch I had set up for you to look at this. Would like to get this in as it's a pre-req for many of the other changes (or at least the other changes would be different if we were to choose not to do this). |
} | ||
} | ||
|
||
public class EntityConfiguration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have pulled this and the other XXXConfiguration classes out into separate .cs files in a future PR.
@bricelam @divega @rowanmiller - pinging... |
Assembly.Load(new AssemblyName("System.Core, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")))); | ||
_references.Add(MetadataReference.CreateFromAssembly( | ||
Assembly.Load(new AssemblyName("Microsoft.CSharp, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a")))); | ||
AddReferenceFromName("EntityFramework.Relational.Design"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these right for .NET Core?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment this supports net451, dnx451 and dnxcore50. I haven't updated for netcore451 yet. Let me do that as a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lajones Is there some docs or blog post explaining all these new target farmework variants? (too many, IMHO)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bricelam Nope, it's not right for anything bull full CLR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ErikEJ I don't know of one but I'll try to get you an answer for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ErikEJ You may have already read it, but Introducing .NET Core has a lot of great information in it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't most of the templating code be in Commands, not Relational.Design? |
I don't mind if we move the Templating part into Commands. It could be useful if something else Command-based wants to use templating in the future. And it would set the stage for localizing the actual IO into a service which I had intended to do later for testing purposes anyway. But again let me do that as a separate PR. Thanks for the review. |
|
||
#if DNX451 || DNXCORE50 | ||
var libraryManager = _serviceProvider.GetRequiredService<ILibraryManager>(); | ||
var metadataReference = libraryManager.GetLibraryExport(name).MetadataReferences.Single(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A library can provide multiple metadata references. Packages do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have updated to loop over them instead.
Checked in with commit fd64cf8. |
With this PR Reverse Engineering would use templating (an adjusted-copy of the templating code from Scaffolding) to produce the generated code. And updated to use separate .cshtml files for the templates (rather than inline strings).
Also made the updates which were already made in the release branch (#1928, #1934, #1937 & #1938) and which needed equivalent changes in the template-based code in this branch plus a few minor changes to whitespace and to support API changes that have been made in dev.