-
Notifications
You must be signed in to change notification settings - Fork 678
docs: clarify FileInstallationStore and FileStateStore are dev-only #2446
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
Conversation
lukegalbraithrussell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but am also open to making this a big red callout card if needed
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2446 +/- ##
=======================================
Coverage 93.09% 93.09%
=======================================
Files 40 40
Lines 11240 11240
Branches 713 713
=======================================
Hits 10464 10464
Misses 764 764
Partials 12 12
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
@lukegalbraithrussell Oh, interesting! It's your call, but that could be a good idea to do somewhere in the docs. Would you use the |
zimeg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mwbrooks LGTM and thanks for making this more clear!
I left a few comments on separating sections and think in follow up PRs we can improve the jsdoc for these classes, but please let me know what you think!
Co-authored-by: Luke Russell <[email protected]>
Summary
This pull request addresses a reported security issue where a developer was using the
FileStateStorefor production and reported security vulnerabilities.We already clarify that
FileInstallationStoreis for development only and this PR extends the paragraph to includeFileStateStore.Requirements (place an
xin each[ ])