Skip to content

Fix silly npm warnings #10339

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

Merged
merged 2 commits into from
May 18, 2019
Merged

Fix silly npm warnings #10339

merged 2 commits into from
May 18, 2019

Conversation

BrennanConroy
Copy link
Member

Fixes #10251

@anurse Who does dependency update things now? (This is test only)

@BrennanConroy BrennanConroy added the area-signalr Includes: SignalR clients and servers label May 17, 2019
@analogrelay
Copy link
Contributor

@anurse Who does dependency update things now? (This is test only)

The formal "compliance" stuff is automated in our build through the Component Governance tools. I think @Pilchie should be notified though, just as @Eilon was in the past despite the automatic stuff.

In this case, the dependency is being updated in order to fix NPM warnings during installation (see linked issue) and is used only in test code. I approve ✔️ of this update.

Copy link
Contributor

@analogrelay analogrelay left a comment

Choose a reason for hiding this comment

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

Looks like the ts-jest update barfed on the lock file but c'est la vie.

@BrennanConroy
Copy link
Member Author

Also looks like I forgot to run tests locally, and the update broke some stuff

@Eilon
Copy link
Contributor

Eilon commented May 17, 2019

Yup, please let @Pilchie know about significant OSS updates. For me the most important ones were ones that affect the shipping product (even minor updates). For test/build-only assets I mostly cared about significant changes (e.g. new dependency).

@@ -0,0 +1,12 @@
{
Copy link
Member Author

Choose a reason for hiding this comment

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

ts-jest changed something with how they compiled files which made the types now actually needed, so had to add a new config file to have the correct typeRoots and use that for FunctionalTests

Copy link
Member

Choose a reason for hiding this comment

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

👍

@BrennanConroy
Copy link
Member Author

🆙 📅

@halter73
Copy link
Member

Still approved.

@BrennanConroy BrennanConroy merged commit e953537 into master May 18, 2019
@BrennanConroy BrennanConroy deleted the brecon/npmWarn branch May 18, 2019 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-signalr Includes: SignalR clients and servers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warnings in SignalR build due to invalid peer dependencies
4 participants