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 json type support for SQLite #2814

Merged
merged 2 commits into from Sep 26, 2018
Merged

Conversation

@kibertoad
Copy link
Collaborator

@kibertoad kibertoad commented Sep 20, 2018

Implements #1036

@elhigu
Copy link
Member

@elhigu elhigu commented Sep 25, 2018

This one needs also integration test, which makes sure that the create query works against actual DB.

@kibertoad
Copy link
Collaborator Author

@kibertoad kibertoad commented Sep 25, 2018

@elhigu Looks like there is one already, see test/integration/schema/index.js:

it('allows adding a field', function() {
        return knex.schema.table('test_table_two', function(t) {
          t.json('json_data', true);
        });
      });

It used to pass before when text was passed and it doesn't seem to have started failing now.

@elhigu elhigu merged commit 776a115 into knex:master Sep 26, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.003%) to 85.717%
Details
@bkniffler
Copy link

@bkniffler bkniffler commented Nov 20, 2018

This is awesome, can this be released to @next? Thanks!

@kibertoad
Copy link
Collaborator Author

@kibertoad kibertoad commented Nov 20, 2018

@bkniffler next-3 was released on Sep 27, are you sure it doesn't include it?
(we do plan to release next-4 very soon anyway, though)

@kibertoad kibertoad deleted the JakeGinnivan:feature/1036-sqlite-json branch Nov 20, 2018
@bkniffler
Copy link

@bkniffler bkniffler commented Nov 20, 2018

Ah, I had next-2 installed since according to package.json it is the latest, at least on master.

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

3 participants