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

Migrations Design: Invalid 'using' statements for default namespace #2467

Closed
natemcmaster opened this Issue Jun 24, 2015 · 16 comments

Comments

Projects
None yet
10 participants
@natemcmaster
Member

natemcmaster commented Jun 24, 2015

Invalid using ; added to generated migration file because this simple project has no default namespace.

Setup https://gist.github.com/natemcmaster/90e2388c0ee1d29f440a.

Using dnx 1.0.0-beta6-12114 , EF 7.0.0-beta6-13584

dnx . ef migration add Initial

produces 20150624223653_Initial.Designer.cs which includes this at the top of the file:

using System;
using Microsoft.Data.Entity;
using Microsoft.Data.Entity.Metadata;
using Microsoft.Data.Entity.Relational.Migrations.Infrastructure;
using ;

The same using block appears in ModelSnapshot.cs

@natemcmaster natemcmaster added cross-plat and removed cross-plat labels Jun 24, 2015

@bricelam

This comment has been minimized.

Show comment
Hide comment
@bricelam

bricelam Jun 26, 2015

Member

Related to #2261

Member

bricelam commented Jun 26, 2015

Related to #2261

@softmarshmallow

This comment has been minimized.

Show comment
Hide comment
@softmarshmallow

softmarshmallow Jul 23, 2017

this issue still occurs, and all entity under Proj.Models....

softmarshmallow commented Jul 23, 2017

this issue still occurs, and all entity under Proj.Models....

@Rick-Anderson

This comment has been minimized.

Show comment
Hide comment
@Rick-Anderson

Rick-Anderson Aug 18, 2017

FWIW, more customers will be hitting this now that we're creating more CLI docs. See aspnet/Scaffolding#599

Rick-Anderson commented Aug 18, 2017

FWIW, more customers will be hitting this now that we're creating more CLI docs. See aspnet/Scaffolding#599

@ajcvickers

This comment has been minimized.

Show comment
Hide comment
@ajcvickers

ajcvickers Aug 18, 2017

Member

@Rick-Anderson Why do we have sample projects with no default namespace?

Member

ajcvickers commented Aug 18, 2017

@Rick-Anderson Why do we have sample projects with no default namespace?

@Rick-Anderson

This comment has been minimized.

Show comment
Hide comment
@Rick-Anderson

Rick-Anderson Aug 18, 2017

@ajcvickers maybe this is rare. When you follow the instructions at aspnet/Scaffolding#599, you get no namespace. The project has a namespace, not the DC.
It's not clear the CLI requires you to add a namespace for the DC.

Rick-Anderson commented Aug 18, 2017

@ajcvickers maybe this is rare. When you follow the instructions at aspnet/Scaffolding#599, you get no namespace. The project has a namespace, not the DC.
It's not clear the CLI requires you to add a namespace for the DC.

@ajcvickers

This comment has been minimized.

Show comment
Hide comment
@ajcvickers

ajcvickers Aug 18, 2017

Member

@Rick-Anderson It may not be required, but it's certainly best practice. If we have some guidance or tooling that is by default creating classes not in any namespace, then I think the real thing to address is that guidance and tooling. (Think about what VS does--every project you create or class you add to it is in a namespace by default; it's idiomatic to .NET.)

Member

ajcvickers commented Aug 18, 2017

@Rick-Anderson It may not be required, but it's certainly best practice. If we have some guidance or tooling that is by default creating classes not in any namespace, then I think the real thing to address is that guidance and tooling. (Think about what VS does--every project you create or class you add to it is in a namespace by default; it's idiomatic to .NET.)

@Rick-Anderson

This comment has been minimized.

Show comment
Hide comment
@Rick-Anderson

Rick-Anderson Aug 18, 2017

@ajcvickers does the bug belong to .NET CLI for not warning on -dc MovieContext in the following command?

dotnet aspnet-codegenerator razorpage -m Movie -dc MovieContext -udl -outDir Pages\Movies --referenceScriptLibraries

Rick-Anderson commented Aug 18, 2017

@ajcvickers does the bug belong to .NET CLI for not warning on -dc MovieContext in the following command?

dotnet aspnet-codegenerator razorpage -m Movie -dc MovieContext -udl -outDir Pages\Movies --referenceScriptLibraries

@ajcvickers

This comment has been minimized.

Show comment
Hide comment
@ajcvickers

ajcvickers Aug 18, 2017

Member

@Rick-Anderson I don't know enough about how the templating works to know what part should be ensuring a namespace. Maybe ask whoever authored the templates?

Member

ajcvickers commented Aug 18, 2017

@Rick-Anderson I don't know enough about how the templating works to know what part should be ensuring a namespace. Maybe ask whoever authored the templates?

@Rick-Anderson

This comment has been minimized.

Show comment
Hide comment
@Rick-Anderson

Rick-Anderson Aug 18, 2017

@ajcvickers But it is generating invalid code. You're not required to have a NS. It seems like it would be better to fix the producer of bad generated code than fixing the inputs.

Rick-Anderson commented Aug 18, 2017

@ajcvickers But it is generating invalid code. You're not required to have a NS. It seems like it would be better to fix the producer of bad generated code than fixing the inputs.

@ajcvickers

This comment has been minimized.

Show comment
Hide comment
@ajcvickers

ajcvickers Aug 18, 2017

Member

@Rick-Anderson We didn't close this--we are intending to fix it. We will re-triage the priority. I still believe the bigger issue is that something out of Microsoft if generating a class without a namespace. That's something we should just not do.

Member

ajcvickers commented Aug 18, 2017

@Rick-Anderson We didn't close this--we are intending to fix it. We will re-triage the priority. I still believe the bigger issue is that something out of Microsoft if generating a class without a namespace. That's something we should just not do.

@ajcvickers ajcvickers removed this from the Backlog milestone Aug 18, 2017

@Rick-Anderson

This comment has been minimized.

Show comment
Hide comment
@Rick-Anderson

Rick-Anderson Aug 18, 2017

@ajcvickers I'm guessing this is low priority FWIW.
I sometimes create VS projects and remove the namespace - just to cut down on noise/indenting on rendered code. Even if the scaffolder forces a namespace, nothing stops the user from removing the namespace. I'll shut up now.

Rick-Anderson commented Aug 18, 2017

@ajcvickers I'm guessing this is low priority FWIW.
I sometimes create VS projects and remove the namespace - just to cut down on noise/indenting on rendered code. Even if the scaffolder forces a namespace, nothing stops the user from removing the namespace. I'll shut up now.

@ajcvickers ajcvickers added this to the 2.1.0 milestone Aug 23, 2017

@bricelam bricelam changed the title from Invalid 'using' statements generated by migration scaffolding (default namespace) to Migrations Design: Invalid 'using' statements for default namespace Sep 8, 2017

@julielerman

This comment has been minimized.

Show comment
Hide comment
@julielerman

julielerman Oct 24, 2017

Would love to see this problem go away. Yes, every class should have a namespace but this is a rough punishment. ;)

julielerman commented Oct 24, 2017

Would love to see this problem go away. Yes, every class should have a namespace but this is a rough punishment. ;)

@smitpatel

This comment has been minimized.

Show comment
Hide comment
@smitpatel

smitpatel Oct 24, 2017

Contributor

Where empty string is being injected. Just need a check there before printing out using statement for it.

Contributor

smitpatel commented Oct 24, 2017

Where empty string is being injected. Just need a check there before printing out using statement for it.

smitpatel added a commit that referenced this issue Nov 8, 2017

Design: Put system namespaces first in generated code
Don't generate empty namespace if context is not in namespce

Resolves #10225
Resolves #2467

@smitpatel smitpatel assigned smitpatel and unassigned bricelam Nov 8, 2017

smitpatel added a commit that referenced this issue Nov 9, 2017

Design: Put system namespaces first in generated code
Don't generate empty namespace if context is not in namespce

Resolves #10225
Resolves #2467
@BennyM

This comment has been minimized.

Show comment
Hide comment
@BennyM

BennyM Nov 17, 2017

Awesome, I was just about to clone the repo and fix this :)

BennyM commented Nov 17, 2017

Awesome, I was just about to clone the repo and fix this :)

@bricelam

This comment has been minimized.

Show comment
Hide comment
@bricelam

bricelam Nov 17, 2017

Member

@BennyM I love the enthusiasm! Feel free to check out some of our other up-for-grabs issues if you're still interested in submitting a PR. 😉

Member

bricelam commented Nov 17, 2017

@BennyM I love the enthusiasm! Feel free to check out some of our other up-for-grabs issues if you're still interested in submitting a PR. 😉

@BlinkSun

This comment has been minimized.

Show comment
Hide comment
@BlinkSun

BlinkSun Nov 28, 2017

oh didnt see that #2467 !!! Thank you !!!

BlinkSun commented Nov 28, 2017

oh didnt see that #2467 !!! Thank you !!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment