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

Reverse Engineer: Cmd parameter to disable naming logic #6018

Closed
rowanmiller opened this issue Jul 7, 2016 · 12 comments
Closed

Reverse Engineer: Cmd parameter to disable naming logic #6018

rowanmiller opened this issue Jul 7, 2016 · 12 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@rowanmiller
Copy link
Contributor

We have the ability to create a custom service that handles column => property naming (see https://gist.github.com/natemcmaster/353f4ab3efb4514eeaec846df28f0e24). However, some folks may want to opt out of our default logic and just have property names line up with column names. We think this makes sense to have as a top level parameter to the commands.

BTW we should have a design discussion about whether this literally just uses the column name, or makes some basic attempts to land with a valid C# identifier.

@ajcvickers
Copy link
Member

A lot of people have been asking for this, so we should try to do it for 2.0.

@ajcvickers ajcvickers modified the milestones: 2.0.0-preview1, 2.0.0 Apr 19, 2017
@JuanIrigoyen
Copy link

In the previous version, I used the solution proposed by natemcmaster for regenerate my model https://gist.github.com/natemcmaster/353f4ab3efb4514eeaec846df28f0e24,

In the next version I used a modified version proposed by
https://romiller.com/2017/02/10/ef-core-1-1-pluralization-in-reverse-engineer/

But now with the last version nothing of the two works, who can I regenerate my model for disable name-changing logic.

@smitpatel
Copy link
Member

@JuanIrigoyen - Solution posted by Nate should still work.

@lajones
Copy link
Contributor

lajones commented Jun 14, 2017

@ajcvickers @smitpatel @bricelam @divega Do we need to design this? I was getting going on a simple boolean flag --use-database-names which would switch this on. But do we want to consider a future where users can e.g. name a CandidateNamingService which we then search the project for? Even if we don't do the search right now it might argue for a --use-naming-service [service_name] flag instead (where for right now we only recognize PascalCase and UseDatabaseNames naming services)?

@divega
Copy link
Contributor

divega commented Jun 14, 2017

@lajones can we bring this to the design meeting tomorrow?

On first thought I like the idea of adding a very simple flag that maps directly to what seems to be a common need, e.g. --no-pascal-case.

Bringing your own naming service already requires coding the service (or at least installing a NuGet package containing it), so I think it might be ok for that scenario to always require some tinkering with design time services.

But even if we later decide to do something in the command line to allow specifying custom naming services it won't necessarily obsolete the flag: We would just need to decide what happens when the user
specifies both settings.

BTW, Rowan mentioned in the original post that there we should decide whether the non-PascalCase option should still do some sanitization to land on valid C# identifiers. I tend to believe it should. Then maybe it makes sense to pass the state of the --no-pascal-case flag in the GenerateCandidateIdentifier method. Custom candidate naming services would be free to decide what to do with it (e.g. honor it, ignore it, or throw if it is set in a way that isn't supported).

@lajones
Copy link
Contributor

lajones commented Jun 15, 2017

Discussed in design mtg today. Outcome: we will use the simple boolean flag --use-database-names with no argument. It will imply a) that the candidate name we use will be just the database name as it was passed to us, and b) that we should make no attempt to pluralize/singularize names as we currently do.

There was also the suggestion that we move the existing singularize/pluralize step to before the uniquify step (currently it's after and though we haven't come across issues with this they are theoretically possible).

@lajones
Copy link
Contributor

lajones commented Jun 29, 2017

Fixed with #8989 - commit dc7d379.

@lajones lajones closed this as completed Jun 29, 2017
@divega divega added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jun 30, 2017
@piyey
Copy link

piyey commented Sep 13, 2017

Did anybody see fieldnames are still Pascalcased?

@ajcvickers
Copy link
Member

@piyey Can you file a new issue with full details if the flag is not doing what you expect?

@piyey
Copy link

piyey commented Sep 14, 2017

Ok @ajcvickers , I will search if someone else reported this issue.

@camelinckx
Copy link

Hi @piyey, did we file an issue for this, I can confirm the flag does not activate the feature? (i.e. field names are still PascalCased)

@bricelam
Copy link
Contributor

bricelam commented Oct 4, 2017

#9820

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

No branches or pull requests

9 participants