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

Allow suppression of OnConfiguring() generation. #20811

Merged
merged 6 commits into from
May 27, 2020
Merged

Conversation

lajones
Copy link
Contributor

@lajones lajones commented May 1, 2020

Fixes #20526
Fixes #19899.

Allow suppression of OnConfiguring() generation.

@lajones lajones requested a review from bricelam May 1, 2020 20:18
@lajones lajones self-assigned this May 1, 2020
Fix for 19608. Truncate/Uniquify Sequence Names.
@lajones
Copy link
Contributor Author

lajones commented May 6, 2020

Pinging @bricelam

src/EFCore.Design/Design/OperationExecutor.cs Outdated Show resolved Hide resolved
src/EFCore.Tools/tools/EntityFrameworkCore.psm1 Outdated Show resolved Hide resolved
src/ef/Commands/DbContextScaffoldCommand.Configure.cs Outdated Show resolved Hide resolved
src/ef/Commands/DbContextScaffoldCommand.cs Outdated Show resolved Hide resolved
src/ef/Properties/Resources.resx Outdated Show resolved Hide resolved
src/EFCore.Design/Properties/DesignStrings.resx Outdated Show resolved Hide resolved
Comment on lines 110 to 112
else
{
_reporter.WriteWarning(DesignStrings.OnConfiguringWarning);
Copy link
Contributor

@bricelam bricelam May 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
else
{
_reporter.WriteWarning(DesignStrings.OnConfiguringWarning);
else if (!codeOptions.SuppressOnConfiguring)
{
_reporter.WriteWarning(DesignStrings.SensitiveInformationWarning);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other words, we shouldn't discourage OnConfiguring. Instead, we should just encourage Name=.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bricelam OK. Going ahead with this. What I think you're saying is we should have the --no-onconfiguring flag, but we should not point people towards it as a way of solving the problem - instead we should just mention Name= (and leave them to find --no-onconfiguring on their own if that's what they really want).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. This is the decision we made in triage.

@lajones lajones requested a review from bricelam May 27, 2020 05:09
@lajones
Copy link
Contributor Author

lajones commented May 27, 2020

@bricelam All updated as requested. I'd appreciate the final review so I can get this in. Thx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants