Skip to content

Simplify NpgsqlMigrationsSqlGenerator.Generate for CreateSequenceOperation - #639

Merged
roji merged 1 commit into
npgsql:devfrom
slavanap:simplify_generate
Sep 23, 2018
Merged

Simplify NpgsqlMigrationsSqlGenerator.Generate for CreateSequenceOperation#639
roji merged 1 commit into
npgsql:devfrom
slavanap:simplify_generate

Conversation

@slavanap

@slavanapslavanap commented Sep 16, 2018

Copy link
Copy Markdown
Contributor

Simple refactoring for #607

@roji, let's continue our discussion here

builder.AppendLine(_sqlGenerationHelper.StatementTerminator);
EndStatement(builder);
base.Generate(operation, model, builder);
operation.ClrType = oldValue;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a bit far-fetched, but I'd rather we didn't set ClrType if we don't need to (i.e. when on PG 10.0 or above). In other words, if VersionAtLeast(10, 0) just call directly into base.Generate() and return. Otherwise looks good, thanks for the correction...

@slavanap

Copy link
Copy Markdown
ContributorAuthor

I've amended commit with the proposed fix.

@rojiroji left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, thanks!

@roji
roji merged commit 9a40b14 into npgsql:devSep 23, 2018
@roji

roji commented Sep 23, 2018

Copy link
Copy Markdown
Member

Note - not backporting to 2.1 because we don't have the backwards compat support there. I think this is an OK situation, 2.2 isn't so far away in any case.

@slavanap

Copy link
Copy Markdown
ContributorAuthor

Okay. Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@slavanap@roji