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

Deparsing ALTER TABLE #30

Merged
merged 7 commits into from
Aug 25, 2015
Merged

Deparsing ALTER TABLE #30

merged 7 commits into from
Aug 25, 2015

Conversation

JackDanger
Copy link
Contributor

I've done a couple things today:

  • Began extracting some of the complex parts of the deparser into lib/deparser/{filename}.rb (just the ALTER TABLE and INTERVAL stuff now, because that's the most complex).
  • Added support for deparsing ALTER TABLE statements and all of the various clauses that can appear with in that.

Tests included for everything, I'm keeping the tests like you set them up, @lfittl, so they just round-robin through the parser and deparser. This means anybody can refactor to their hearts content if they don't like the structure of the code :)

This is a precursor to separating the parts of the deparser.
@lfittl
Copy link
Member

lfittl commented Aug 25, 2015

Awesome - great work!

Will go through these changes in detail but looks good from a first look :)

@JackDanger
Copy link
Contributor Author

I cleaned up the commits a bit. I apologize in advance for the size of this PR, it includes slightly more change than I'd normally put into one. I made it pretty straightforward to review commit-by-commit so that should make things easier. Most of the changes are actually just moved files or having to indent whole files to appease Rubocop.

@lfittl
Copy link
Member

lfittl commented Aug 25, 2015

👍 Lets merge it in!

lfittl added a commit that referenced this pull request Aug 25, 2015
@lfittl lfittl merged commit ae54ca4 into pganalyze:master Aug 25, 2015
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