Fix 885: Make loading of dependencies always synchronous #894
Conversation
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
added 8 commits
September 16, 2025 10:02
…d at install time. Convert default to sync loading. Remove InstallContext as its no longer needed.
… valid. Remove command line module name from dependency loads (only needed for initial check when running load/update)
isc-kiyer
requested review from
isc-dchui,
isc-egabhart,
isc-eneil,
isc-jili,
isc-jlechtne,
isc-pbarton and
isc-tleavitt
as code owners
isc-dchui
reviewed
Oct 24, 2025
| // If it does, always use it | ||
| // Particularly relevant for compilation of dependencies | ||
| // The "clean" phase can be used to bypass this (by removing modules from the current namespace) | ||
| // Forcing an update from disk can also override this, although we ensure that the same module is |
There was a problem hiding this comment.
Nit: could also remove this comment about forcing an update from disk since it looks like it refers to tForceSnapshotReload
| // Check for valid license key prior to top-level module load | ||
| // This may throw an exception. | ||
| if '$data($$$ZPMHandledModules($namespace)) { | ||
| set tLicenseIsValid = ..CheckLicenseKey() |
There was a problem hiding this comment.
Do we not need to check the license key anymore? If so, could probably just remove the ClassMethod entirely
| } | ||
| } else { | ||
| // Common: name, required, description | ||
| // Common to bother modifier and parameter |
|
Just wondering out loud if there is a way to attach "doc" to code changes on Github (probably not). But a reminder to update the Wiki with the new CLI changes once live! |
isc-jili
reviewed
Oct 24, 2025
| ### Fixed | ||
|
|
||
| ### Security | ||
|
|
There was a problem hiding this comment.
Sorry but I think my recent merge to main has made your CHANGELOG stale so would require another pull from main
isc-jili
reviewed
Oct 24, 2025
| set tResults(tCommands,"modifiers","tree")="" | ||
|
|
||
| set tCommands($increment(tCommands)) = "install SomeModule" | ||
| set tCommands($increment(tCommands)) = "install SomeModule -synchronous" |
There was a problem hiding this comment.
Does this check that there's a warning displayed for the deprecation?
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 of changes:
GetPackageService()is called which slows down buildsresolves #885