Skip to content

Conversation

maghis
Copy link
Contributor

@maghis maghis commented Nov 13, 2016

Improves on #1220 and adds stricter compiler options as suggested by @blakeembrey

@coveralls
Copy link

coveralls commented Nov 13, 2016

Coverage Status

Coverage remained the same at 88.09% when pulling fe130dd on maghis:patch-1 into 9d5881e on aws:master.

@blakeembrey
Copy link

LGTM 👍 Do you think the rootDir should be changed just in case some .d.ts files are missed? Not sure if that's an issue here, but looks great 😄

@maghis
Copy link
Contributor Author

maghis commented Nov 13, 2016

@blakeembrey I didn't encounter any issue because of the rootDir but I have tried just a small subset. This is so much better than the funky types I was using before 😄

@blakeembrey
Copy link

blakeembrey commented Nov 13, 2016

Sounds good. I'm excited to be using it myself too, once it's working for me 😄 I only brought up rootDir because I believe the root is treated at where the tsconfig.json file is located otherwise (but I could be wrong, there's too many ways to resolve and I can't remember how that one works because I don't really use it with -p).

@chrisradek
Copy link
Contributor

@maghis
Thanks for the PR! I'll merge to master now. I'm going to try and tackle the other TypeScript issues today, but if I run out of time I'll include this with the next npm publish and follow-up with the rest.

@chrisradek chrisradek merged commit 02586f5 into aws:master Nov 14, 2016
@maghis
Copy link
Contributor Author

maghis commented Nov 14, 2016

@blakeembrey ok, I tested and it looks like it doesn't make any difference.

For reference, I tested by adding an invalid line in one of the root .d.ts and the test failed as expected.

@lock
Copy link

lock bot commented Sep 28, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants