-
Notifications
You must be signed in to change notification settings - Fork 359
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
feat: zip support for bundled app references [DHIS2-16755] #16455
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #16455 +/- ##
=========================================
Coverage 66.71% 66.72%
- Complexity 31791 31799 +8
=========================================
Files 3531 3531
Lines 130868 130868
Branches 15245 15245
=========================================
+ Hits 87311 87320 +9
+ Misses 36435 36429 -6
+ Partials 7122 7119 -3
Flags with carried forward coverage won't be shown. Click here to find out more. see 8 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
@amcgee this uses a very simple approach where the URL is just checked for the It would also be useful to be able to have rolling references to e.g. "latest" and then determine the version from the App Hub API. As it is it just supports explicit versions - but that is still very useful for the release candidates and final releases. We might wish to accompany this with signing packages (i.e. signing the packages at built time and publishing the signature files on App Hub/GitHub, then verifying the signatures during the bundling process. |
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.
Very delayed review. Generally this looks good and quite straight-forward. I added one comment about cleaning up straggling files in case of an error
} | ||
|
||
} catch (err) { | ||
throw err |
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.
(very delayed) I think we should remove any generated files here - for example, if the app is successfully extracted to temp_path but the webapp.manifest file isn't valid json the require
will throw and you'll end up with an orphaned directory
This PR has not seen any activity in the last 5 months. The PR will be closed in 30 days if the stale label is not removed. Please note that this is an automated message and we might very well be the reason why there has not been any activity lately. Please remove the stale label if you would like to continue working on the PR. Make sure that you have requested a review by a dev or a team https://github.com/orgs/dhis2/teams. |
Allows apps-to-bundle.json to reference zip files.
(examples from either App Hup [app-management-app], or GitHub release [capture-app])
We lose a little information on branches and commits, so the bundled-apps.json ends up like this (which I think is OK):
I get the app name and build date from the manifest.webapp.