This repository has been archived by the owner. It is now read-only.

docs(webpack): fix for tsconfig.json provided in example #2892

Merged
merged 3 commits into from Dec 1, 2016

Conversation

Projects
None yet
4 participants
@skryvets
Contributor

skryvets commented Nov 28, 2016

Pull request for this issue.

@googlebot

This comment has been minimized.

googlebot commented Nov 28, 2016

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.
1 similar comment
@googlebot

This comment has been minimized.

googlebot commented Nov 28, 2016

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@googlebot googlebot added the CLA: no label Nov 28, 2016

@googlebot

This comment has been minimized.

googlebot commented Nov 28, 2016

CLAs look good, thanks!

@googlebot googlebot added CLA: yes and removed CLA: no labels Nov 28, 2016

@skryvets

This comment has been minimized.

Contributor

skryvets commented Nov 28, 2016

I signed it!

@Foxandxss

This comment has been minimized.

Member

Foxandxss commented Nov 28, 2016

We don't need exclude without files and why do we need typeRoots? I am totally in for the lib, I wanted to add that with an update but forgot.

@Foxandxss Foxandxss self-assigned this Nov 28, 2016

@skryvets

This comment has been minimized.

Contributor

skryvets commented Nov 28, 2016

Hello @Foxandxss ,

  1. I've added exclude in order to not process all files inside node_modules.
  2. I thought that typeRoots actually can help solve the issue. But I've just verified and add "lib": ["es2015", "dom"], is enough.

Sorry. for this.
As far as I understand we just need to add "lib": ["es2015", "dom"],. Can I modify it and submit updated version in this pull request?

Best regards,
Siarhei

@Foxandxss

This comment has been minimized.

Member

Foxandxss commented Nov 28, 2016

Sure you can.

The node_modules exclusion is only needed if you specify the files by hand, since we do not, node_modules is excluded automagically.

@skryvets

This comment has been minimized.

Contributor

skryvets commented Nov 28, 2016

@Foxandxss,
Thank you for the explanation. I didn't know about that.
I've updated my changes.

Best regards,
Siarhei

@@ -6,8 +6,9 @@
"sourceMap": true,
"emitDecoratorMetadata": true,
"experimentalDecorators": true,
"lib": ["es2015", "dom"],
"removeComments": false,

This comment has been minimized.

@wardbell

wardbell Nov 30, 2016

Contributor

Would you please remove this removeComments setting too. That is the default setting and we don't want it here. I thought I'd removed it everywhere but I guess I missed this one.

Then I can merge your PR.

@skryvets

This comment has been minimized.

Contributor

skryvets commented Dec 1, 2016

Hello @wardbell ,

I've just removed removeComments setting and pushed my changes.
Sorry for the delay with this.

Best regards,
Siarhei

@Foxandxss Foxandxss merged commit 18981f9 into angular:master Dec 1, 2016

2 checks passed

cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Foxandxss

This comment has been minimized.

Member

Foxandxss commented Dec 1, 2016

Thank you.

@skryvets skryvets deleted the skryvets:webpack-guide-tsconfig-fix branch Dec 1, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.