-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Socket.dev reports on all our repos #17
Comments
I don't really see any downside aside from the noise from comments. (Which shouldn't be just noise if it's doing anything useful.) And we can always remove it. |
The one thing I am not sure is how well it reports when we dont have lockfiles. There are good reasons not to have lock files for libraries, but it might be another thing to consider at some point. If we can get reliable automation around testing before publish with fully updated locks then ideally we can add them, but for now even just something to help tell us that a PR like this did not pull in anything surprising would be nice. |
Yes! 100%
We can do a test and if this is an issue we can revert the integration or explore alternatives. |
IME with these tools, the only way to know for sure is to fork a repo and compare the results with and without a lock file. |
Now that I am thinking about it I think I have this running on some without lockfiles and it is working well. Examples: wesleytodd/create-git#54 (comment) |
Seems like we are good to add it. We can do a fast check with the team in the next TC meeting and enable it |
Yep I will add the agenda label. But yeah then I can enable it on a few to start. |
I think we should add the Socket.dev tooling to all three orgs repos. It is easy to do, I have already done it for some personal repos and some work ones. I find it is the best in class for warning me about changes I might care about in updated dependencies. Anyone opposed to this? I could add it just to one for the time being if we wanted to try it out first.
cc @expressjs/express-tc @expressjs/security-wg
The text was updated successfully, but these errors were encountered: