Add installationMethod as optional output from installFexPackage#52
Add installationMethod as optional output from installFexPackage#52ehennestad merged 2 commits intomainfrom
Conversation
Test Results21 tests 21 ✅ 21s ⏱️ Results for commit 3668e7d. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Pull request overview
Adds an optional second output (installationMethod) to matbox.setup.internal.installFexPackage so callers can distinguish between zip-extracted installs (“folder”) and MATLAB-managed add-on installs (“mltbx”).
Changes:
- Updated
installFexPackagesignature to return[packageTargetFolder, installationMethod]. - Set
installationMethodto"folder"for/zipdownloads and"mltbx"for/mltbxinstalls and already-installed add-ons. - Updated output documentation and adjusted mltbx
packageTargetFoldertostring(missing)for the install path.
| if isInstalled | ||
| matlab.addons.enableAddon(toolboxIdentifier, version) | ||
| packageTargetFolder = toolboxFolder; | ||
| installationMethod = "mltbx"; |
There was a problem hiding this comment.
When isInstalled is true, the function assigns outputs but never applies the ~nargout/nargout==1 clearing logic used in the download path. This leads to inconsistent behavior when calling installFexPackage(...) without output arguments (e.g., it may return/echo values in the already-installed case). Consider moving the nargout handling to a single place at the end of the function so it applies to both branches.
| % Output: | ||
| % packageTargetFolder - Path to the installed package folder, or | ||
| % string(missing) for mltbx packages (managed by MATLAB) | ||
| % installationMethod - "folder" for zip-extracted packages, | ||
| % "mltbx" for MATLAB toolbox packages |
There was a problem hiding this comment.
The docstring says packageTargetFolder is string(missing) for mltbx packages, but when the add-on is already installed you return toolboxFolder (which can be "" if the internal lookup fails). This makes the output contract inconsistent across the two mltbx paths; consider either always returning string(missing) for mltbx, or updating the docs and normalizing toolboxFolder=="" to string(missing) before returning.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #52 +/- ##
==========================================
- Coverage 42.26% 41.55% -0.72%
==========================================
Files 59 59
Lines 1455 1468 +13
==========================================
- Hits 615 610 -5
- Misses 840 858 +18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add
installationMethod("folder" or "mltbx") as second optional output frommatbox.setup.internal.installFexPackage