-
Notifications
You must be signed in to change notification settings - Fork 19
Update templates #381
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
Update templates #381
Conversation
radu-matei
left a comment
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 requires a rebase now to resolve conflicts.
Thanks!
8819202 to
be39d64
Compare
Signed-off-by: Karthik Ganeshram <karthik.ganeshram@fermyon.com>
Signed-off-by: Karthik Ganeshram <karthik.ganeshram@fermyon.com>
Add a readme note on how to use the debugger. Also adds a commented out outbound host for debugger connection and a npm script to build debug. Signed-off-by: Karthik Ganeshram <karthik.ganeshram@fermyon.com>
be39d64 to
1de996e
Compare
tschneidereit
left a comment
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.
Looks great, thank you!
The only comments I left inline are about whether to switch from spin up to spin watch, which might resolve some issues with the debugger, but for all I know might also introduce others. If you haven't already, I think testing that would be good.
| "componentRuntime": { | ||
| "executable": "spin", | ||
| "options": [ | ||
| "up", |
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.
one thing to consider here is to change this to watch. That'd probably require/enable other changes to the lifecycle, as the process wouldn't need to be restarted when content changes.
| "componentRuntime": { | ||
| "executable": "spin", | ||
| "options": [ | ||
| "up", |
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.
Same comment as for the http-js template
| "componentRuntime": { | ||
| "executable": "spin", | ||
| "options": [ | ||
| "up", |
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.
... and same here
|
@tschneidereit Thoughts on landing this as is and I can test the |
|
I also think we would need build profiles to tightly integrate with the |
Oh, that's a good point! Yeah, with that in mind I agree that landing this as-is makes sense. |
|
This makes me realize that it'd be valuable to have a |
|
|
You're not: I had forgotten that exists! @karthik2804 might be useful to update the templates to use that, since it'd allow us to avoid having to restart |
|
I have a feeling that |
I need to verify that the debugger works.