Add CDS sources using the CLI #32
Conversation
| }), | ||
| force: flags.boolean({ | ||
| description: 'Do not fail if a file already exist and overwrite it.' | ||
| }), | ||
| help: flags.help({ char: 'h' }) | ||
| }; | ||
|
|
||
| static args = [ | ||
| { | ||
| name: 'projectDir', |
There was a problem hiding this comment.
I don't think this is necessary. In init I added this as you may create a new directory. Here you need to have a dir already otherwise it doesn't make sense. That's why the --projectDir is currently hidden (mainly there for tests and to quickly enable it if we find out it's needed).
There was a problem hiding this comment.
But is it an issue? I would argue either we have it or we don't. So either remove the flag or keep both for consistency (and make it non-hidden, I guess). What do you think?
There was a problem hiding this comment.
My assumption was that a simpler (public) API makes it easier to understand the docs. So omitting a flag/arg that is not useful for 99% of users (and can be simply worked around by cd projectDir) would be a positive change.
But I have not validated that assumption, so feel free to change my mind on this.
There was a problem hiding this comment.
I have removed all the --projectDir flags as discussed, but I decided to leave the argument, because I think it makes our commands consistent. I don't think projectDir makes the API hard to understand, it is still very simple, while the change is tiny.
There was a problem hiding this comment.
To throw my 2cts in, I also have no idea whether we need it or not. So the question is how do find that out in the future? Otherwise we'll have the same discussion on the next PR.
| // // Reduce stock of books upon incoming orders | ||
| // srv.before ('CREATE', 'Orders', async (req) => { | ||
| // const tx = cds.transaction(req), order = req.data; | ||
| // if (order.Items) { |
src/commands/init.ts
Outdated
| this.log('| |'); | ||
| this.log('| 🚀 Next steps: |'); | ||
| private printSuccessMessage(isScaffold: boolean, addCds: boolean) { | ||
| const message = ['✅ Init finished successfully.', '', '🚀 Next steps:']; |
There was a problem hiding this comment.
I think the complexity of this function is tied to the combinatorial complexity of the input space (aka the more combinations of input you can have, the more complex it will be to determine what needs to be printed).
If there are things that exclude each other (e.g. scaffold vs no scaffold), that probably needs to be decided upfront. For everything else, maybe a more declarative approach can help, i.e. to simply collect all the message fragments for the input flags/args.
| }), | ||
| force: flags.boolean({ | ||
| description: 'Do not fail if a file already exist and overwrite it.' | ||
| }), | ||
| help: flags.help({ char: 'h' }) | ||
| }; | ||
|
|
||
| static args = [ | ||
| { | ||
| name: 'projectDir', |
There was a problem hiding this comment.
To throw my 2cts in, I also have no idea whether we need it or not. So the question is how do find that out in the future? Otherwise we'll have the same discussion on the next PR.
Proposed Changes
Adds a command and init flag to enable the creation of a CAP compatible project.
Calling
sap-cloud-sdk init --addCdsin an empty directory will create a nest app with an additional module that exposes the CDS example catalogue service, as well as a.cdsrcand thedbandsrvdirectories. I also adds dependencies and scripts to the package.json and changes the.gitignore. The same command called within an exiting project will create the same resources except for the additional example nest module (as we don't know whether nest is used).sap-cloud-sdk add-cdsagain does the same but without the standardinitparts.Limitations: This does not consider deploying on CF. We have to clarify with the CAP colleagues what the best practice is here.
Checklist
Further Comments
If the changes are complex, please discuss what alternatives you have evaluated and why you picked this particular solution. Please make sure to highlight any trade-offs that we should know about.