fix: Update Adp writer deploy scripts #2362
Conversation
🦋 Changeset detectedLatest commit: 2b1a2b6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| }) | ||
| }; | ||
|
|
||
| if (!isAppStudio()) { |
There was a problem hiding this comment.
Could you always add this please, so that projects are ready for usage everywhere.
Even if you run in AppStudio, you have the url and auth type. The url is part of the destination configuration and the authtype is always reentranceTicket since we only add the task for ABAP cloud systems.
There was a problem hiding this comment.
For BAS we don't need this configuration, because we are using destinations.
There was a problem hiding this comment.
A project generated in BAS and then checked into e.g. github, could be checked out in VSCode and it should also work there as well. Just because it is generated in BAS, does not necessarily mean it only used in BAS.
| }, | ||
| "scripts": { | ||
| "build": "ui5 build --exclude-task generateFlexChangesBundle generateComponentPreload minify --clean-dest", | ||
| "start": "<%= locals.options?.fioriTools ? 'fiori run' : 'ui5 serve' %> --open /test/flp.html#app-preview", | ||
| "start-editor": "<%= locals.options?.fioriTools ? 'fiori run' : 'ui5 serve' %> --open /test/adaptation-editor.html"<%if (locals.deploy) {%>, | ||
| "deploy": "ui5 build --config ui5-deploy.yaml --exclude-task <%= locals.options?.fioriTools ? 'deploy-to-abap' : 'abap-deploy-task' %> generateFlexChangesBundle generateComponentPreload --clean-dest && <%= locals.options?.fioriTools ? 'fiori deploy' : 'deploy' %> --config ui5-deploy.yaml", | ||
| "undeploy": "<%= locals.options?.fioriTools ? 'fiori undeploy' : 'undeploy' %> --config ui5-deploy.yaml --lrep \"apps/<%= app.reference %>/appVariants/<%= app.id %>/\"", | ||
| "deploy": "npm run build && fiori deploy --config ui5-deploy.yaml && rimraf archive.zip", |
There was a problem hiding this comment.
The deploy process should not generate an archive.zip if not explicitly requested, therefore, I am confused that you require rimraf here.
If it does generate an archive.zip then there is a regression that needs an investigation.
There was a problem hiding this comment.
You also need to add the IF again because you only have the fiori deploy command if the Fiori tools are added, otherwise, you need to use ui5 build
There was a problem hiding this comment.
@tobiasqueck in SAP Fiori tools, the package.json deploy script is appended as follows;
"deploy": "npm run build && fiori deploy --config ui5-deploy.yaml && rimraf archive.zip",
During the deploy task, there is a zip generated on the fly. However, a user can specify a path to an already generated zip which will obviously bypass this zip generation task.
https://github.com/SAP/open-ux-tools/blob/main/packages/deploy-tooling/src/cli/archive.ts#L82
There was a problem hiding this comment.
I checked in "@sap-ux/abap-deploy-config-writer" module and it is always generating the same scripts like in this PR.
There was a problem hiding this comment.
Well ... I still don't understand the usage of rimraf because as @longieirl said, the zip is generated on the fly, so nothing needs to be deleted.
The deploy config generated is only generating things for the Fiori tools, while the adp writer already supports both, Fiori tools and a "pure" open source version, so instead of introducing a regression in the adp-writer, this needs to be improved in the deploy-config writer, and then, maybe reused here instead of copying an implementation
There was a problem hiding this comment.
@tobiasqueck the archive remains if there is any exception thrown during the process! I think this is more of a cleanup task.
| }, | ||
| \\"scripts\\": { | ||
| \\"build\\": \\"ui5 build --exclude-task generateFlexChangesBundle generateComponentPreload minify --clean-dest\\", | ||
| \\"start\\": \\"fiori run --open /test/flp.html#app-preview\\", | ||
| \\"start-editor\\": \\"fiori run --open /test/adaptation-editor.html\\", | ||
| \\"deploy\\": \\"ui5 build --config ui5-deploy.yaml --exclude-task deploy-to-abap generateFlexChangesBundle generateComponentPreload --clean-dest && fiori deploy --config ui5-deploy.yaml\\", | ||
| \\"undeploy\\": \\"fiori undeploy --config ui5-deploy.yaml --lrep \\\\\\"apps/the.original.app/appVariants/my.test.app/\\\\\\"\\", | ||
| \\"deploy\\": \\"npm run build && fiori deploy --config ui5-deploy.yaml && rimraf archive.zip\\", |
There was a problem hiding this comment.
.gitignore should also exclude the archive.zip from source control which I think from looking at the snapshots, is appended i.e. *.zip
| }, | ||
| "scripts": { | ||
| "build": "ui5 build --exclude-task generateFlexChangesBundle generateComponentPreload minify --clean-dest", | ||
| "start": "<%= locals.options?.fioriTools ? 'fiori run' : 'ui5 serve' %> --open /test/flp.html#app-preview", | ||
| "start-editor": "<%= locals.options?.fioriTools ? 'fiori run' : 'ui5 serve' %> --open /test/adaptation-editor.html"<%if (locals.deploy) {%>, | ||
| "deploy": "ui5 build --config ui5-deploy.yaml --exclude-task <%= locals.options?.fioriTools ? 'deploy-to-abap' : 'abap-deploy-task' %> generateFlexChangesBundle generateComponentPreload --clean-dest && <%= locals.options?.fioriTools ? 'fiori deploy' : 'deploy' %> --config ui5-deploy.yaml", | ||
| "undeploy": "<%= locals.options?.fioriTools ? 'fiori undeploy' : 'undeploy' %> --config ui5-deploy.yaml --lrep \"apps/<%= app.reference %>/appVariants/<%= app.id %>/\"", | ||
| "deploy": "npm run build && <%= locals.options?.fioriTools ? 'fiori deploy' : 'deploy' %> --config ui5-deploy.yaml && rimraf archive.zip", |
There was a problem hiding this comment.
This is quite a change. Before your change, the the build configuration from the ui5-deploy.yaml was used for building the sources, now it is using the ui5.yaml. Are you sure about that?
So, maybe something like npm run build -- --config ui5-deploy.yaml && <%= locals.options?.fioriTools ? 'fiori deploy' : 'deploy' %> --config ui5-deploy.yaml && rimraf archive.zip is the better option.
There was a problem hiding this comment.
Well we don't need ui5-deploy.yaml for build, we need it only for deployment.
|
fix for #2361
Update deployment related scripts generated from the writer in @sap-ux/adp-tooling for Adp Projects