-
Notifications
You must be signed in to change notification settings - Fork 10
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: add types for cds.build
#266
base: main
Are you sure you want to change the base?
Conversation
48fb209
to
e370dc7
Compare
Hi Ludwig, thanks for this contribution! We're actually currently looking into how we can properly offer cds-dk types, hopefully without tainting the core cds types and causing too much confusion. I see that you have pointed out that the types are only available from dk in the comments, but I'd still like to put this on hold until we have figured some things out. Best, |
Hi Daniel, sure thing. It's not that high on my requirements list. I only created this PR because of the typescript issues in When you figured out how and where to add the dk-specific typings, just give a quick shout out, I am happy to do it 😊 Regards, |
Hi @hakimio , yes, absolutely! Going for DT was considered, but finally discarded for now for two reasons:
While we would generally love to move in with DT, these drawbacks deterred us from doing so at the current stage. We might revisit this decision in the future. Best, |
@daogrady That's not how
I think you should definitely reconsider this decision since it was made on a false premise. |
Also, the symlink workaround doesn't work reliably with any package manger:
|
Hi @hakimio ,
That is all part of "handing over control". I am not debating whether these restrictions set up by DT are good (because I think they are). It is just a matter of feasibility.
Again, we are aware of the bumps our current solution causes and the convenience moving in with DT would bring. Which is why we are likely to revisit this decision at some point. But not in the foreseeable future.
Is working with the Best, |
Maybe @chgeo can help review the PRs
Maybe you could just run them in another GH repo and use a bot to open PRs for DT repo.
While for you, as the maintainer, it might be convenient to make breaking changes in minor version releases, I, as the user of the library, really hate when that happens (yes, I know that the lib is in "alpha" state). The change would improve DX and would be highly appreciated by all the lib users.
While
Yes, it is a valid workaround, but it's a custom undocumented workaround and it's not only me who is having issues with the symlink solution. Soon there will be more and more people who are not happy with it. It's just not a good solution by any means and having to come up with your own workarounds is a pretty bad DX, imho.
Don't want to take more of your valuable time with this discussion, just hope this happens sooner than later. |
Adds types for
cds.build
to have proper type support when writing a custom CDS build plugin