Skip to content

Feature/codegen script #78

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

Closed

Conversation

grumpyTofu
Copy link

Added functionality to use codegen from script instead of from cli (mimics cli functionality)

Relates to issue: #63

@grumpyTofu
Copy link
Author

@phryneas npm test doesn't seem to pass all tests on this branch or main. Is it okay merge this PR?

@phryneas
Copy link
Collaborator

Hey @grumpyTofu, I'm just back home after being gone after a week, sorry for the late replay.

On first glance, this will probably not work since the interfaces suggested in #63 were merely a suggestion on how options could look like, but nothing taking options in that shape is implemented internally yet.

@grumpyTofu
Copy link
Author

No worries! Everyone needs a break every now and then!

You are correct; I did re-read 63 and these changes wouldn't necessarily cover anything listed there. So perhaps that wasn't the proper issue to tag. The changes would only address #77 and allow us to run from a script instead of CLI which is great if you want to use environment variables. Would you be open to incorporating this PR to add that functionality? Or would you rather have all the options built out that you mentioned in the other issue?

@phryneas
Copy link
Collaborator

Hm, good question.
If we expose something now, we will add an api and then immediately add a breaking change when all the options to it change. So we kinda have to do at least some of the work laid out there and then could re-start this maybe with a subset of the final api, so we can extend towards the final api instead of breaking towards it.

@phryneas
Copy link
Collaborator

phryneas commented Nov 1, 2021

@grumpyTofu #81 should be pretty much runnable by now and also export a scriptable api. I'm gonna close this over here now - could you please try #81 out and give feedback over there?

@phryneas phryneas closed this Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants