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

Design: Modernize generated C# #14545

Open
Tracked by #22946
bricelam opened this issue Jan 29, 2019 · 18 comments
Open
Tracked by #22946

Design: Modernize generated C# #14545

bricelam opened this issue Jan 29, 2019 · 18 comments

Comments

@bricelam
Copy link
Contributor

bricelam commented Jan 29, 2019

This is related to #11744, but more general. We should investigate what the minimum version of C# we need to support in EF Core 3.0 is and modernize our generated code. I'm pretty sure there are a few expression bodied methods I'd like add...

@roji
Copy link
Member

roji commented Jan 29, 2019

I think that with #10347 we'd be generating classes that use C# 8 nullability to express requiredness in properties... So I think C# 8 should be fine?

@ajcvickers
Copy link
Member

Default is currently "8.0 (beta)"

@ajcvickers ajcvickers added this to the 3.0.0 milestone Feb 4, 2019
@ajcvickers
Copy link
Member

Consider:

  • Reverse engineering to non-nullable types (strings).
  • Can we do something based on project configuration?

@roji
Copy link
Member

roji commented Feb 4, 2019

See nullability-related notes here: #10347 (comment)

@bricelam
Copy link
Contributor Author

bricelam commented May 9, 2019

I think we can safely assume C# 8.0 as our language minimum version in EF Core 3.0. This is the default for projects targeting .NET Standard 2.1 and .NET Core 3.0.

@bricelam
Copy link
Contributor Author

bricelam commented May 9, 2019

Areas I'd like to improve:

@roji
Copy link
Member

roji commented May 9, 2019

There's #15520 for tracking NRT scaffolding, should we close that and track everything there? Note that for NRTs specifically there's the additional opt-in flag so it's an additional thing to do.

@bricelam
Copy link
Contributor Author

bricelam commented May 9, 2019

I think it goes beyond DbContext scaffolding too—model snapshots and migrations may also have to be updated when NRT is enabled in the project

@bricelam
Copy link
Contributor Author

bricelam commented May 9, 2019

But maybe not as much as I think since NRT is more about member definitions than types...

@roji
Copy link
Member

roji commented May 9, 2019

Yeah, could be...

@bricelam
Copy link
Contributor Author

For example, will these work as-is in a project that enables NRTs?

DbContext

  • protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
  • protected override void OnModelCreating(ModelBuilder modelBuilder)

Migration

  • protected override void Up(MigrationBuilder migrationBuilder)
  • protected override void Down(MigrationBuilder migrationBuilder)
  • protected override void BuildTargetModel(ModelBuilder modelBuilder)

ModelSnapshot

  • protected override void BuildModel(ModelBuilder modelBuilder)

@ajcvickers ajcvickers modified the milestones: 3.0.0, Backlog May 10, 2019
@bricelam bricelam modified the milestones: Backlog, 3.1.0 Aug 21, 2019
@bricelam bricelam added the good first issue This issue should be relatively straightforward to fix. label Aug 29, 2019
@ajcvickers ajcvickers modified the milestones: 3.1.0, Backlog Sep 4, 2019
@Eli-Black-Work
Copy link

Would it make sense to reopen #25223? I feel like this issue is much broader than that one 🙂

@bricelam
Copy link
Contributor Author

bricelam commented Jun 7, 2022

We like broad issues. We consider everything together when working on it. If, for whatever reason, it doesn't all fit into one release, we prioritize the important parts and file separate issues for any remaining items.

@Eli-Black-Work
Copy link

Okay 🙂

I'd like to post my request here, then, since I don't think it's been mentioned in this thread:

When EF Core starts modernizing its generated C# code, I'd love for it to respect our .editorconfig code-style settings.

For example, csharp_style_namespace_declarations = file_scoped/block_scoped should be respected 🙂

@DamianEdwards
Copy link
Member

When EF Core starts modernizing its generated C# code, I'd love for it to respect our .editorconfig code-style settings.

There's been similar requests for ensuring code generated by templates does this too. In VS new files from templates are formatted according to current .editorconfig preferences. In the case of the CLI and tools like dotnet new we still need to figure out the mechanics of how this would work (i.e. discovering the Roslyn bits and loading/executing them, detecting dotnet format and running that, etc.).

@AraHaan
Copy link
Member

AraHaan commented Jul 29, 2022

Oh that should actually be relatively simple if roslyn was made into it's own shared framework (then it could use it without problems).

Pros:

  • Any application that uses roslyn could roll forward if they use it as a shared framework.
  • Visual Studio could use it as well.
  • Source Generators and analyzers could use the shared framework as well (which could reduce restore times for large analyzers/generators) (Less downloads because the roslyn packages (would be stored in $DOTNET_ROOT/packs) and shared framework (would be stored in $DOTNET_ROOT/shared) would be cached on the user's machine).

Cons:

  • Any applications that use apis that end up with breaking changes might break, if they set their application to roll forward on the frameworks.

bricelam added a commit to bricelam/efcore that referenced this issue Jul 29, 2022
This refactors our existing scaffolding code into T4 templates that we can both precompile to use as our default code generator and also ship somehow (probably `dotnet new`) as a starting point for users to start customizing.

Part of dotnet#4038, part of dotnet#14545, fixes dotnet#25473, resolves dotnet#25546, resolves dotnet#25547, fixes dotnet#27087, part of dotnet#27588, fixes dotnet#28187
@bricelam bricelam removed this from the 7.0.0 milestone Jul 29, 2022
bricelam added a commit to bricelam/efcore that referenced this issue Aug 1, 2022
This refactors our existing scaffolding code into T4 templates that we can both precompile to use as our default code generator and also ship somehow (probably `dotnet new`) as a starting point for users to start customizing.

Part of dotnet#4038, part of dotnet#14545, fixes dotnet#25473, resolves dotnet#25546, resolves dotnet#25547, fixes dotnet#27087, part of dotnet#27588, fixes dotnet#28187
bricelam added a commit to bricelam/efcore that referenced this issue Aug 8, 2022
This refactors our existing scaffolding code into T4 templates that we can both precompile to use as our default code generator and also ship somehow (probably `dotnet new`) as a starting point for users to start customizing.

Part of dotnet#4038, part of dotnet#14545, fixes dotnet#25473, resolves dotnet#25546, resolves dotnet#25547, fixes dotnet#27087, part of dotnet#27588, fixes dotnet#28187
bricelam added a commit to bricelam/efcore that referenced this issue Aug 9, 2022
This refactors our existing scaffolding code into T4 templates that we can both precompile to use as our default code generator and also ship somehow (probably `dotnet new`) as a starting point for users to start customizing.

Part of dotnet#4038, part of dotnet#14545, fixes dotnet#25473, resolves dotnet#25546, resolves dotnet#25547, fixes dotnet#27087, part of dotnet#27588, fixes dotnet#28187
@ajcvickers ajcvickers added this to the Backlog milestone Aug 11, 2022
@bricelam bricelam removed their assignment Jul 8, 2023
@glen-84
Copy link

glen-84 commented Sep 14, 2023

There's been similar requests for ensuring code generated by templates does this too.

This is probably dotnet/templating#5505.

@glen-84
Copy link

glen-84 commented Sep 15, 2023

Can we drop the BOM 💣now as well?

@ajcvickers ajcvickers removed the good first issue This issue should be relatively straightforward to fix. label May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants