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

Make TS stubs modern #3080

Merged
merged 1 commit into from Mar 5, 2019

Conversation

Projects
None yet
3 participants
@felixmosh
Copy link
Contributor

felixmosh commented Mar 3, 2019

Small refactor to the TS stubs, so they will use arrow functions and es6 import / export.

t.increments();
t.timestamps();
});
};
}

This comment has been minimized.

Copy link
@kibertoad

kibertoad Mar 3, 2019

Collaborator

Why remove semicolons, though?

This comment has been minimized.

Copy link
@elhigu

elhigu Mar 3, 2019

Collaborator

Normal function declaration shouldn't have semicolon there. With arrow function, it would be obligatory though.

This comment has been minimized.

Copy link
@kibertoad

kibertoad Mar 5, 2019

Collaborator

@felixmosh So if I understand correctly, semicolons should be restored, since it's an arrow function?..

This comment has been minimized.

Copy link
@felixmosh

felixmosh Mar 5, 2019

Author Contributor

@kibertoad , no, up is not an arrow function :]
The function that we pass to createTable is an arrow func, and there are semi-colons there.

This comment has been minimized.

Copy link
@kibertoad

kibertoad Mar 5, 2019

Collaborator

Fair enough.

@@ -1,9 +1,9 @@
import * as Knex from "knex";

exports.seed = function (knex: Knex): Promise<any> {
export async function seed(knex: Knex): Promise<any> {

This comment has been minimized.

Copy link
@kibertoad

kibertoad Mar 3, 2019

Collaborator

if it returns promise anyway, why make it async?

This comment has been minimized.

Copy link
@elhigu

elhigu Mar 3, 2019

Collaborator

To allow using await inside the function.

This comment has been minimized.

Copy link
@kibertoad

kibertoad Mar 3, 2019

Collaborator

Await inside the function is kinda broken, though, not sure we want to encourage it: #2581

This comment has been minimized.

Copy link
@elhigu

elhigu Mar 3, 2019

Collaborator

Yes we do :) That is really strange though. It doesn't make sense at all that it works when calling then(), but not with await... Maybe we have missed returning a promise from .then somewhere 🤔

This comment has been minimized.

Copy link
@kibertoad

kibertoad Mar 3, 2019

Collaborator

I'll try writing a failing test for that case.

@kibertoad kibertoad merged commit f3f0750 into tgriesser:master Mar 5, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 85.09%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.