-
Notifications
You must be signed in to change notification settings - Fork 35
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: Support multiple services for ui5 yaml files and manifest.json #2455
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: e0e2e94 The changes in this PR will be included in the next version bump. This PR includes changesets to release 33 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 |
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.
Hi @broksy,
I've tested adding a mockserver and found following issues:
In interactive mode there is no text for overwrite, it just gives the technical information questions.overwrite › (y/N)
:
✔ questions.pathToMock › mainService: /sap/opu/odata4/dmo/sb_travel_mdsk_o4/srvd/dmo/sd_travel_mdsk/0001/
? questions.overwrite › (y/N)
I can see a maintained text "Overwrite existing services", but can't see it in the question when running on command line.
After mockserver config has been added to a project, the empty filewebapp/localService/data/keep
is stored in that project. Is this intentionally? I don't think we should add the empty keep
file. We can create the folder empty, but in my opinion it is then for the developer to decide, what to do with the empty folder.
There is an issue with the unit tests in @sap-ux/mockserver-config-writer
"webapp/localService/data/keep": Object { | ||
"contents": "", | ||
"state": "modified", | ||
}, |
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 think we should not add empty keep
files. If a module relies on the fact the folder webapp/localService/data
exists, we need to make it more robust.
"webapp/localService/data/keep": Object { | ||
"contents": "", | ||
"state": "modified", | ||
}, |
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 don't think we should generate empty keep files into a project.
packages/mockserver-config-writer/src/mockserver-config/index.ts
Outdated
Show resolved
Hide resolved
packages/mockserver-config-writer/src/mockserver-config/index.ts
Outdated
Show resolved
Hide resolved
packages/mockserver-config-writer/src/mockserver-config/ui5-mock-yaml.ts
Outdated
Show resolved
Hide resolved
packages/mockserver-config-writer/test/fixtures/ui5-mock-config/ui5.yaml
Outdated
Show resolved
Hide resolved
I found the issue with i18n, there was a conflict due to global initialization of i18n in |
@@ -71,7 +71,7 @@ export interface OdataService { | |||
/** | |||
* Annotations can either be EDMX annotations or CDS annotations. | |||
*/ | |||
annotations?: EdmxAnnotationsInfo | CdsAnnotationsInfo; | |||
annotations?: EdmxAnnotationsInfo[] | CdsAnnotationsInfo[]; |
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.
This is a breaking type change. @sap/fiori-generator
will need to be updated to accomodate, or better if you continue to support backward compatible single entries
Quality Gate passedIssues Measures |
Allows to add multiple services in YAML file configurations and Manifest (and multiple annotations per service).
Previous logic only rewrote whole mockserver config. This is still possible, but through an optional parameter.
During mockserver config generation
localService/data
folder (mockdataPath) is also generated if it does not exist in project.