cloudspannerecosystem / wrench

wrench - Schema management tool for Cloud Spanner -
MIT License
239 stars 46 forks source link

Add comment support #15 #32

Closed mookjp closed 2 years ago

mookjp commented 4 years ago

WHAT

WHY

googlebot commented 4 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

mookjp commented 4 years ago

@googlebot I signed it!

googlebot commented 4 years ago

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

mookjp commented 4 years ago

I will think about the better approach like #15 and left this PR until then!

vvakame commented 3 years ago

@110y how about this PR? I need comment support 😉

halmai commented 3 years ago

I would also need to add comments to the scripts but unfortunately, a regexp-based solution is not reliable enough.

For example, if there is a command like this in the script:

INSERT INTO mytable (field) VALUES ('-- hello --')

it will become wrong:

INSERT INTO mytable (field) VALUES ('

illustration: https://play.golang.org/p/28ELu7ugA2D

I think a more careful parsing is needed.

I am happy to implement it but don't want to take it out from your hand either. Please, let me know if you have time for it, otherwise I will start working on it.

Another note: maybe not everybody is happy with a breaking change how wrench works. Therefore, I would be more conservative and would add a command-line option to turn ON the comment removal. The default behaviour should be the original behaviour.

110y commented 2 years ago

@mookjp @halmai

Sorry for tooooo late, but now wrench support writing comments to DDL/DML files by this PR: https://github.com/cloudspannerecosystem/wrench/pull/54, which parses DDL/DML and ignore comments by using spansql.

So, let me close this PR, thank you so much 🙏