Skip to content

Conversation

@Stefan-Puskarica
Copy link
Contributor

Currently the generator only generates one newline between statements which leads to code like this:

CREATE TABLE [MyTable] (
  ...
);
ALTER TABLE [MyTable]
  ...;

Which can be unfavourable, I've added a script generator option that allows you to specify how many newlines you want so you can get something like this instead:

CREATE TABLE [MyTable] (
  ...
);

ALTER TABLE [MyTable]
  ...;

@Stefan-Puskarica
Copy link
Contributor Author

@microsoft-github-policy-service agree

Copy link
Member

@llali llali left a comment

Choose a reason for hiding this comment

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

the new option is not added to SqlScriptGeneratorOptions so this will have build error.

@llali
Copy link
Member

llali commented Oct 9, 2024

Please also add tests to verify the new option

@Stefan-Puskarica
Copy link
Contributor Author

the new option is not added to SqlScriptGeneratorOptions so this will have build error.

Hi, I'm not sure what you mean, it was added to the .xml file. As far as I can see there is no tracked .cs equivalent to be generated rather it is generated at build time. When I built and tested locally it worked for me

@Stefan-Puskarica
Copy link
Contributor Author

Please also add tests to verify the new option

Where should this go? I don't see any current tests for SQL generation

@Stefan-Puskarica
Copy link
Contributor Author

@llali can I get an update on this?

@llali
Copy link
Member

llali commented Nov 8, 2024

the new option is not added to SqlScriptGeneratorOptions so this will have build error.

Hi, I'm not sure what you mean, it was added to the .xml file. As far as I can see there is no tracked .cs equivalent to be generated rather it is generated at build time. When I built and tested locally it worked for me

you are right my bad.

@llali
Copy link
Member

llali commented Nov 8, 2024

@llali can I get an update on this?

Please add test for this new option. Thanks

@Stefan-Puskarica
Copy link
Contributor Author

@llali can I get an update on this?

Please add test for this new option. Thanks

I've added a test

Sign up for free to 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