Skip to content

Conversation

@gfx
Copy link
Contributor

@gfx gfx commented Dec 14, 2017

@types/* is not required for ts-node users. It just adds installation time.

@coveralls
Copy link

coveralls commented Dec 14, 2017

Coverage Status

Coverage remained the same at 77.427% when pulling 5d0aedb on gfx:move_types_to_devdeps into 1541dfe on TypeStrong:master.

@blakeembrey
Copy link
Member

If you can add tests to ensure this is true, I'd be happy to merge it.

@blakeembrey
Copy link
Member

See #475 also.

@blakeembrey
Copy link
Member

Will merge and make a quick release now, but it'd be good if you could submit tests so this doesn't break anyone.

@blakeembrey
Copy link
Member

Also, although they are not "runtime" libraries, that doesn't mean they aren't used by people depending on this library. Any types can be transitively exposed by TypeScript, so they would usually be required as a dependency and not a dev dependency. The only reason not to do this, that I can think of, is dependency size. If these deps were on NPM and not GitHub (a separate issue, I can't contribute these definitions easily so never have) I'm not sure it'd be a problem. And if it were, this is really an issue with TypeScript - it shouldn't make dependency management harder than using node.js.

@blakeembrey blakeembrey merged commit f04c5f7 into TypeStrong:master Dec 15, 2017
@gfx
Copy link
Contributor Author

gfx commented Dec 15, 2017

Added tests in 3f66c44

@gfx
Copy link
Contributor Author

gfx commented Dec 15, 2017

Wow! PR is merged while I was writing tests 😆

Will make a PR for tests

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.

3 participants