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

Add inherits to postgres. Updated dialect-specific methods. #601

Merged
merged 1 commit into from
Mar 16, 2016

Conversation

joeframbach
Copy link
Contributor

@bendrucker
Copy link
Member

Can you explain the MyISAM related stuff?

@joeframbach
Copy link
Contributor Author

The engine stuff was added long ago (0f38f04) but I couldn't find a unit test around it. I added two tests for "engine": 1. Make sure MySql uses "engine". 2. Make sure PostgreSQL ignores "engine".

Adding "inherits" to PostgreSQL was similar to "engine", so I followed the same principles.

@bleupen
Copy link

bleupen commented Jan 29, 2015

+1

@rogerzklotz
Copy link

Any chance this is going to be added? I'd be happy to make any changes that are needed to get issues resolved, if no one else can/wants to.

@joeframbach
Copy link
Contributor Author

@MrBucket I've updated to resolve conflicts.

@rogerzklotz
Copy link

@joeframbach Thank you for updating.
@bendrucker Is there any chance this can be merged into master? I think Joe or I would be happy to make any changes necessary to make this happen. Thank you!

@rhys-vdw
Copy link
Member

rhys-vdw commented Feb 2, 2016

@MrBucket @joeframbach Hadn't seen this one before. I'll merge this if we can remove the /lib folder that is no longer under version control. Sorry for the back and forth!

@elhigu
Copy link
Member

elhigu commented Feb 15, 2016

I started going through old pull requests and get them merged if possible or close otherwise if they are not going to be finished.

@joeframbach Nice feature! If you rebase this one more time to top of master, I'll make sure this gets merged.

@elhigu
Copy link
Member

elhigu commented Mar 15, 2016

@MrBucket If you still like to finish this, rebasing this to top of master and removing code from /lib should be enough to get this merged. Code did look good.

I'll leave this open for one more month and if still pending, close the PR and move this to issues to wait for finishing.

@joeframbach joeframbach force-pushed the postgres_inherits branch 2 times, most recently from 6dcd9be to fcb92a7 Compare March 15, 2016 23:36
@joeframbach
Copy link
Contributor Author

I've updated but Travis seems confused. Gonna close and re-open to kick off Travis.

@joeframbach joeframbach reopened this Mar 15, 2016
elhigu added a commit that referenced this pull request Mar 16, 2016
Add inherits to postgres. Updated dialect-specific methods.
@elhigu elhigu merged commit 4189968 into knex:master Mar 16, 2016
@elhigu
Copy link
Member

elhigu commented Mar 16, 2016

@joeframbach Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants