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

@joeframbach joeframbach commented Dec 18, 2014

@bendrucker
Copy link
Member

@bendrucker bendrucker commented Dec 18, 2014

Can you explain the MyISAM related stuff?

@joeframbach
Copy link
Contributor Author

@joeframbach joeframbach commented Dec 18, 2014

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 bleupen commented Jan 29, 2015

+1

@rogerzklotz
Copy link

@rogerzklotz rogerzklotz commented Aug 17, 2015

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 joeframbach force-pushed the joeframbach:postgres_inherits branch from 2b696c9 to 20e721b Sep 1, 2015
@joeframbach
Copy link
Contributor Author

@joeframbach joeframbach commented Sep 1, 2015

@MrBucket I've updated to resolve conflicts.

@joeframbach joeframbach force-pushed the joeframbach:postgres_inherits branch from 20e721b to c4a5790 Sep 1, 2015
@rogerzklotz
Copy link

@rogerzklotz rogerzklotz commented Feb 2, 2016

@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 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 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 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 joeframbach:postgres_inherits branch 2 times, most recently from 6dcd9be Mar 15, 2016
@joeframbach joeframbach force-pushed the joeframbach:postgres_inherits branch to cf8222d Mar 15, 2016
@joeframbach
Copy link
Contributor Author

@joeframbach joeframbach commented Mar 15, 2016

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
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@elhigu
Copy link
Member

@elhigu 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
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants