Conversation
ckadner
left a comment
There was a problem hiding this comment.
Thanks Phu! I have a few comments, questions below
608e30d to
a51c6e5
Compare
d24d5f1 to
41b830f
Compare
ckadner
left a comment
There was a problem hiding this comment.
Thank you Phu I found a few small problem that still need to be addressed
|
|
||
|
|
||
| def verify_npm_packages(): | ||
| check_outdated = run("npm outdated", cwd="./dashboard/origin-mlx/", shell=True) |
There was a problem hiding this comment.
I wonder if we should limit this function to only report vulnerabilities, as opposed to checking for outdated versions?
If we want to run this during our CI, I want to make sure that a new PR only gets a failed check ❌ on a PR, if there is an actual vulnerability that needs to be addressed.
Signed-off-by: Phu Thai <phuthai450@gmail.com>
41b830f to
4c6f3ff
Compare
ckadner
left a comment
There was a problem hiding this comment.
Thanks @BluThaitanium -- just need to add the return code, so that Make can fail if there are outdated packages
aefeee4 to
799f76d
Compare
Signed-off-by: Phu Thai <phuthai450@gmail.com>
799f76d to
e969821
Compare
ckadner
left a comment
There was a problem hiding this comment.
Assigning to me for now since I still need to test it
|
I finally took the time to give your PR another try. I struggled for an hour with Node/npm. Seems like the latest |
|
I do however get this error: [mlx_ckadner] (pr-233 *)$ make check_npm_packages
Package Current Wanted Latest Location Depended by
@material-ui/core 3.9.4 3.9.4 4.12.3 node_modules/@material-ui/core origin-mlx
@material-ui/icons 3.0.2 3.0.2 4.11.2 node_modules/@material-ui/icons origin-mlx
@types/jest 24.0.11 24.0.11 27.0.2 node_modules/@types/jest origin-mlx
@types/js-cookie 2.2.7 2.2.7 3.0.0 node_modules/@types/js-cookie origin-mlx
@types/js-yaml 3.12.7 3.12.7 4.0.4 node_modules/@types/js-yaml origin-mlx
@types/node 14.17.17 14.17.32 16.11.6 node_modules/@types/node origin-mlx
@types/react 16.8.7 16.8.7 17.0.33 node_modules/@types/react origin-mlx
@types/react-dom 16.8.2 16.8.2 17.0.10 node_modules/@types/react-dom origin-mlx
@types/react-router-dom 4.3.5 4.3.5 5.3.2 node_modules/@types/react-router-dom origin-mlx
@types/react-sidebar 3.0.1 3.0.2 3.0.2 node_modules/@types/react-sidebar origin-mlx
@types/styled-components 4.4.3 4.4.3 5.1.15 node_modules/@types/styled-components origin-mlx
codemirror 5.62.3 5.63.3 5.63.3 node_modules/codemirror origin-mlx
js-cookie 2.2.1 2.2.1 3.0.1 node_modules/js-cookie origin-mlx
js-yaml 3.14.1 3.14.1 4.1.0 node_modules/js-yaml origin-mlx
react 16.14.0 16.14.0 17.0.2 node_modules/react origin-mlx
react-codemirror2 5.1.0 5.1.0 7.2.1 node_modules/react-codemirror2 origin-mlx
react-dom 16.14.0 16.14.0 17.0.2 node_modules/react-dom origin-mlx
react-markdown 4.3.1 4.3.1 7.1.0 node_modules/react-markdown origin-mlx
react-router-dom 4.3.1 4.3.1 5.3.0 node_modules/react-router-dom origin-mlx
react-scripts 3.4.4 3.4.4 4.0.3 node_modules/react-scripts origin-mlx
reactjs-popup 1.5.0 1.5.0 2.0.5 node_modules/reactjs-popup origin-mlx
reactstrap 8.10.0 8.10.1 9.0.0 node_modules/reactstrap origin-mlx
remark-gfm 2.0.0 2.0.0 3.0.0 node_modules/remark-gfm origin-mlx
styled-components 4.4.1 4.4.1 5.3.3 node_modules/styled-components origin-mlx
typescript 3.9.10 3.9.10 4.4.4 node_modules/typescript origin-mlx
npm WARN audit request to https://registry.npmjs.org/-/npm/v1/security/audits/quick failed, reason: connect ETIMEDOUT 2606:4700::6810:1723:443
npm ERR! audit endpoint returned an error
Traceback (most recent call last):
File "tools/python/verify_npm_packages.py", line 110, in <module>
verify_npm_packages()
File "tools/python/verify_npm_packages.py", line 89, in verify_npm_packages
\rRun {colorText.BLUE}make update_npm_packages{colorText.END} to secure/update\n'''
IndexError: list index out of range
make: *** [check_npm_packages] Error 1
|
On my other laptop I did not get the error. So I am gonna write this off as a fluke :-) |
ckadner
left a comment
There was a problem hiding this comment.
I think your changes are good.
Could you also update the package-lock.json file with your PR?
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BluThaitanium The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Signed-off-by: BluThaitanium <phuthai450@gmail.com>
9e284a1 to
1ce77c9
Compare
Updates the npm packages through the make check_docs_links command.
Phase 1:
Finds outdated npm packages
If outdated npm packages are found, asks to audit
Phase 2:
If auditing, runs npm audit to attempt to update non-breaking changes
Checks if there are outdated packages again
Phase 3:
If there are still outdated packages, manual inspection is needed
Gives steps on how to manually determine which packages are breaking