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

Remove npm audit warnings and tslint deprecated rule warning #485

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

twindual
Copy link

@twindual twindual commented Aug 9, 2019

What:

This update fixes:
-npm audit warnings
-tslint deprecated rule warning
-adds a new contributor
-adds Google firestore package (to use auth package)

Issue number: N/A

@codecov-io
Copy link

codecov-io commented Aug 9, 2019

Codecov Report

Merging #485 into master will decrease coverage by 0.18%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #485      +/-   ##
==========================================
- Coverage    72.1%   71.92%   -0.19%     
==========================================
  Files          65       65              
  Lines         552      552              
  Branches       34       34              
==========================================
- Hits          398      397       -1     
- Misses        144      145       +1     
  Partials       10       10
Impacted Files Coverage Δ
...tarter/src/app/core/animations/route.animations.ts 80% <0%> (-10%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2c8e90...61fea3b. Read the comment docs.

@twindual
Copy link
Author

twindual commented Aug 9, 2019

Wondering if the codecov/project test failed because of different dependencies, as there were no code changes there?

Any ideas?

Copy link
Owner

@tomastrajan tomastrajan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @twindual!

Thanks for the contribution, please see the review comments.

"target": "es2015",
"typeRoots": ["node_modules/@types"],
"lib": ["es2018", "dom"]
"typeRoots": ["node_modules/@types"]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this file have to change?

"@angular/compiler-cli": "~8.0.0",
"@angular/language-service": "~8.0.0",
"@commitlint/cli": "^7.2.1",
"@angular-devkit/build-angular": "^0.801.0",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Builders / CLI sometimes breaks with minors, would keep it to parch version ~

"@ngx-translate/core": "^11.0.1",
"@ngx-translate/http-loader": "^4.0.0",
"bootstrap": "^4.3.1",
"browser-detect": "^0.2.28",
"firebase": "^6.3.5",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't see any reason to add dependencies while not using them, also firebase is currently out of scope of this project, it is meant to be beginners friendly and is already pretty complex...

Also store freeze is not needed with ngrx 8 so please remove all the extra added dependencies...

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