-
Notifications
You must be signed in to change notification settings - Fork 91
Add Sequence to Schema objects #191
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
base: master
Are you sure you want to change the base?
Conversation
This is a start; i don't see much use until it's in at least some parsers and producers though. |
It works with TT. I am thinking of submitting a Pg compatible template. |
Added Pg SQL producer. |
amazing, thanks! |
PostgreSQL parser is done. |
I will add more producers. YAML is almost ready. |
YAML added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cleanup of dead code please, and then there's some discussion to be had in the comments
... instead of quoted strings which have line breaks.
* Add no_comments field to objects, change old tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
basically what i wrote in the comments.
I'm somewhat concerned about needing to add no_comments
in a bunch of tests - what happened there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we put this in a different PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the diff making?
Okay.
attach_comments => $pargs->{attach_comments} | ||
} | ||
); | ||
# warn '$sequence_def:' . Dumper($sequence_def); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
plz remove dead comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why was no_comments
added here? you didn't touch any mysql stuff, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I need to make one small change to accommodate the no_comments
field.
Either no_comments
becomes default in SQL::Translator::Producer::MySQL
or I apply it in the equivalent test.
Otherwise, I haven't changed MySQL stuff.
Would it better to have no_comments
as default value in MySQL Producer?
as an aside, i prefer not surrounding double sigils with braces, but i'm not gonna be that nitpicky right now |
Got it. 👍 |
No description provided.