The kitchen modules are packaged - #844
Merged
Merged
Conversation
src/main.js, src/kot-manager.js and src/order-alert.js all require src/kitchen-announce.js, and order-alert also requires src/kitchen-call.js. Neither was in build.files, so neither was in the installer. main.js requires one of them, so this is not a quiet degradation on a customer machine. It is "Cannot find module" at startup. This is the same failure that took a shop's handsets down this morning - a module written, correct, tested, and packaged where nothing could reach it - and it is the second one today. The check that catches it already exists and already ran: Packaging checks failed on #840 and it was merged anyway. The guard was not missing, it was overruled. Two lines in build.files. Nothing needs an extraResources entry: only main.js, kot-manager.js and order-alert.js require them and all three live in the asar, unlike server.js which runs from the resources root.
Contributor
|
Merged to Try it at https://develop.posnic.io, or run it yourself: git fetch origin develop && git checkout develop
npm install && npm --prefix api install
npm run dev # then http://localhost:3000When you have tested it, say what you did and what happened, and set Reporting that something is broken is as useful as fixing it. It is |
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
1 participant
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.
Caught while building an installer for a live restaurant. The build was already staged and about to be installed.
src/main.js,src/kot-manager.jsandsrc/order-alert.jsall requiresrc/kitchen-announce.js, andorder-alertalso requiressrc/kitchen-call.js. Neither is inbuild.files, so neither is in the installer. I confirmed against the packaged output: both absent, not besideserver.jsand not in the asar.main.jsrequires one of them. That is not a quiet degradation on a customer machine, it is "Cannot find module" at startup.This is the second one today, and the same failure that took a shop's handsets down this morning (#830): a module written, correct, tested, and packaged where nothing could reach it.
The guard was not missing
scripts/check-packaged-modules.jscatches this exactly, runs innpm run check, and runs in CI as "Packaging checks". It failed on #840 and #840 was merged anyway.That check is the only thing standing between a missing module and a dead till. It cost a shop its handsets this morning and it nearly went out on an installer this afternoon. Worth making it a required check on
developso the button cannot be pressed through it.The fix
Two lines in
build.files. NoextraResourcesentry is needed: onlymain.js,kot-manager.jsandorder-alert.jsrequire them, and all three live in the asar — unlikeserver.js, which runs from the resources root, which is what made #830 subtle.node scripts/check-packaged-modules.jsnow reports all 51 local requires packaged. Desktop suite 2832 pass.