-
Notifications
You must be signed in to change notification settings - Fork 22
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
[FEATURE] flexChangesBundler: Merge existing with new flexibility-bundle.json #974
Conversation
alexeng06
commented
Jan 15, 2024
- merge existing level zero changes into the flexibility-bundle.json of UI5 adaptation projects
- merge existing level zero changes into the flexibility-bundle.json of UI5 adaptation projects
"variants": [] | ||
}; | ||
|
||
const dummyWorkspace = createDummyWorkspace(changeList, manifest, flexBundle); |
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.
Maybe workspaceMock would be a good name
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.
I've just a few comments that should be clarified before merging this PR.
PS: Are there any other details to take into account with this change? Should the merge happen in all projects, or depending on a specific UI5 version or other project specific details?
- added description for parameter existingFlexBundle
This change it mainly for the adaptation project to enable merging changes of the existing base app and the new changes from the adaptation project. |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Mostly suggestions, but the JSDoc should match the code
- adjust JSDoc for flexChangesBundler - reduce number of needed JSON.stringify
LGTM /AzurePipelines run |
- remove unnecessary parameter - adjust JSDoc
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
LGTM
…dle.json Merge any existing level zero changes into the flexibility-bundle.json of UI5 adaptation projects #974
Thanks a lot for your contribution. This PR is available with |
…json Merge any existing level zero changes into the flexibility-bundle.json of UI5 adaptation projects Cherry-pick of #974
…json Merge any existing level zero changes into the flexibility-bundle.json of UI5 adaptation projects Cherry-pick of #974
…json Merge any existing level zero changes into the flexibility-bundle.json of UI5 adaptation projects Cherry-pick of #974