Add the VDM generator to the CLI #18
Conversation
src/commands/generate-vdm.ts
Outdated
|
|
||
| export default class GenerateVdm extends Command { | ||
| static description = | ||
| 'Generates a virtual data model (VDM) from a edmx service file definition. For SAP solution you can find these definitions on https://api.sap.com/'; |
There was a problem hiding this comment.
Is this the correct usage of VDM? I thought only the pre-generated one is a "VDM" and self-generated ones are OData clients. @marikaner could you clarify?
| 'Generates a virtual data model (VDM) from a edmx service file definition. For SAP solution you can find these definitions on https://api.sap.com/'; | |
| 'Generates a virtual data model (VDM) from a edmx service file definition. For SAP solutions, you can find these definitions at https://api.sap.com/.'; |
There was a problem hiding this comment.
I'd just use "OData client code" or "OData API client". I'm not sure whether we have an official stance on what is and isn't VDM.
# Conflicts: # package-lock.json # package.json # src/commands/init.ts
# Conflicts: # package-lock.json # package.json
florian-richter
force-pushed
the
generateVDM
branch
from
89bb7fc
to
eaa278e
Compare
|
@FrankEssenberger I have removed the EDMX file you added for the tests from the git history. EDMX files should not be pushed in open source repositories (due to license reasons). Please make sure to pull the repository and verify the file is gone before pushing again! |
# Conflicts: # package-lock.json
Co-Authored-By: Florian Richter <[email protected]>
|
It might be good to add a short paragraph in the You could also consider to link to the generator tutorial? this might be better to be postponed until later once the tutorial is updated |
src/utils/generate-vdm-util.ts
Outdated
| import { flags } from '@oclif/command'; | ||
| import { AlphabetLowercase, AlphabetUppercase } from '@oclif/parser/lib/alphabet'; | ||
| import { IBooleanFlag, IOptionFlag } from '@oclif/parser/lib/flags'; | ||
| import { GeneratorOptions, GeneratorOptions as GeneratorOptionsSDK, generatorOptionsCli as generatorOptionsSDK } from '@sap/cloud-sdk-generator'; |
There was a problem hiding this comment.
There is something going wrong here
test/generate-vdm.spec.ts
Outdated
| */ | ||
|
|
||
| const warn = jest.fn(message => console.log('MOCKED WARNING: ', message)); | ||
| jest.mock('@oclif/command', () => { |
There was a problem hiding this comment.
You can also remove this warn part as this mock is not needed for the generator as far as I can tell.
| break; | ||
| } | ||
| return collected; | ||
| }, []); |
There was a problem hiding this comment.
I love the .reduce 👍 but it could be even more improved:
return Object.entries(allFalse).reduce((collected: string[], [key, value]) => {
switch (typeof allFalse[key as keyof generateVdmUtil.FlagsParsed]) {
case 'string':
return [...collected, `--${key}`, value!.toString()];
case 'boolean':
return generatorOptionsSDK[key as keyof GeneratorOptionsSDK]?.default ? [...collected, `--no-${key}`] : collected;
}
}, []);There was a problem hiding this comment.
I haven't played around with it, but if possible I would try to avoid as keyof generateVdmUtil.FlagsParsed and as keyof GeneratorOptionsSDK. Casts don't really typecheck in most cases, so if the compiler doesnt complain without them, I would leave them out.
| } | ||
| return collected; | ||
| }, []); | ||
| return [...stringArguments]; |
There was a problem hiding this comment.
If you don't just return the result of the reduce above (see comment above), you can just straight return it here. The copying is unnecessary.
| return [...stringArguments]; | |
| return stringArguments; |
| inputDir: path.resolve(projectDir, 'input'), | ||
| outputDir: path.resolve(projectDir, 'output'), | ||
| serviceMapping: path.resolve(projectDir, 'input', 'service-mapping.json') | ||
| }; |
There was a problem hiding this comment.
Small improvement (avoids an if 😉 )
return {
...(Object.keys(generatorOptionsSDK).reduce((prev, current) => {
const value = generatorOptionsSDK[current as keyof GeneratorOptionsSDK];
return value ? { ...prev, [current]: value.default } : prev;
}, {} as any)),
inputDir: path.resolve(projectDir, 'input'),
outputDir: path.resolve(projectDir, 'output'),
serviceMapping: path.resolve(projectDir, 'input', 'service-mapping.json')
};
Proposed Changes
Add the vdm generator to the CLI.
@florian-richter I am very unhappy with the forwarding of the arguments from the CLI to the SDK generator. I would like to make this generic in a sense that a change from the SDK generator is automatically reflected in the CLI. Do you have an idea to acheive this? Because I want to see the value help in the CLI.
The best idea I had was to export the options from the SDK some generic format and write an adpater which casts this list to
yargsoptions for the SDK use andoclifoptions for use within the CLI. This would minimize the double effort, but still there is the question how we add the generator.Type of Changes
Checklist
Breaking Changes
In general avoid breaking changes, but if you think it is necessary give your reasoning in the Further Comments section.