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

replaces npm-run-all with concurrently #988

Closed
wants to merge 1 commit into
base: latest
from

Conversation

Projects
None yet
1 participant
@ChrisBAshton
Copy link
Collaborator

ChrisBAshton commented Nov 26, 2018

Fixes event-stream vulnerability: dominictarr/event-stream#116

Removes dependency which has a compromised subdependency.

  • Tests added for new features
  • Test engineer approval

@ChrisBAshton ChrisBAshton added this to PR In Progress in Articles via automation Nov 26, 2018

@codeclimate
Copy link

codeclimate bot left a comment

The PR diff size of 10390 lines exceeds the maximum allowed for the inline comments feature.

@codeclimate

This comment has been minimized.

Copy link

codeclimate bot commented Nov 26, 2018

Code Climate has analyzed commit 71eb5e2 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (90% is the threshold).

This pull request will bring the total coverage in the repository to 97.0% (0.0% change).

View more on Code Climate.

@ChrisBAshton

This comment has been minimized.

Copy link
Collaborator

ChrisBAshton commented Nov 26, 2018

Build is currently failing because concurrently doesn't exit the server when the other tasks have passed. There is a --success flag that should allow us to describe when the process should exit but I've not had much luck with it.

Have raised: kimmobrunfeldt/concurrently#177 in the meantime.

@ChrisBAshton

This comment has been minimized.

Copy link
Collaborator

ChrisBAshton commented Nov 26, 2018

Another alternative is to stick with npm-run-all but use the NPM resolutions property to lock event-stream to v3.3.4: https://github.com/bbc/web/pull/234/files

EDIT: have raised #990. We can merge that in the meantime. Suggest we continue with this PR afterwards, as it's still worth swapping out for an unaffected library.

@ChrisBAshton ChrisBAshton referenced this pull request Nov 26, 2018

Closed

lock event-stream to 3.3.4 #990

0 of 2 tasks complete
@ChrisBAshton

This comment has been minimized.

Copy link
Collaborator

ChrisBAshton commented Nov 26, 2018

Superseded by #990

Articles automation moved this from PR In Progress to Done Nov 26, 2018

@ChrisBAshton ChrisBAshton deleted the remove-vulnerability branch Nov 26, 2018

@thecodingdude thecodingdude referenced this pull request Nov 27, 2018

Closed

deleted #24663

@ChrisBAshton ChrisBAshton removed this from Done in Articles Jan 9, 2019

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