Merged
Conversation
akolson
approved these changes
Sep 4, 2024
Member
There was a problem hiding this comment.
Changes LGTM! Thanks @nucleogenesis. (Re-reviewing after spotting build failures..)
akolson
reviewed
Sep 4, 2024
package.json
Outdated
| "Firefox ESR" | ||
| ] | ||
| ], | ||
| "volta": { |
Member
There was a problem hiding this comment.
@nucleogenesis, Looks like this change is causing a build failure with the error
error Your lockfile needs to be updated, but yarn was run with
--frozen-lockfile.
Maybe running yarn install and committing the generated changes in the yarn.lock could solve the issue?
akolson
requested changes
Sep 4, 2024
Member
akolson
left a comment
There was a problem hiding this comment.
Hi @nucleogenesis, looks like we are having some build failures, please see my comment on added dependency.
3c5d740 to
4847969
Compare
volta key and version in package.jsonvolta key and version in package.json~
volta key and version in package.json~
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Description of the change(s) you made
I've found myself having tovolta pin node@16a lot - any reason not to pin this?Reviewer guidance
This is for review once merged (as I understand we test this kind of stuff directly on the unstable server?)
Checkboxes
Check that the KDS update does not break checkboxes in general - can select things in Channel editor, clipboard, etc.
Dev Server Smoke test - any issues w/ pinning the volta version this way?