-
Notifications
You must be signed in to change notification settings - Fork 0
feat: initial implementation #1
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
Conversation
FYI - we follow a convention of naming the pull request according to conventional commits. Then when merging, we squash commit and use the pr title as the commit title |
fixed |
mv ${PACKAGE_NAME}*.tgz ${PACKED_NAME} | ||
npm i -g ${PACKED_NAME} |
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.
Why don't you install directly from the tgz?
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.
The cleanup hook needs a consistent name. pack
names the file with the version so it isn't stable.
bin/test-npm.sh
Outdated
# Verify new project can synth | ||
npm run synth | ||
|
||
cd .. |
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.
Is this necessary
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.
It isn't in CI, but it is if you want to run things locally. Because the script cd
s into the generated repo to run synth, the cleanup will fail since it currently assumes cleaning up from the root directory.
Now that I think about it, probably makes sense to save the root directory and return to it in the cleanup script. I'll fix that before merging.
@@ -0,0 +1,52 @@ | |||
{ | |||
"name": "create-functionless", | |||
"version": "0.1.0", |
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.
Any reasoning around this over 0.0.0? I don't have strong opinions
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.
semantic release will use 1.0.x
as the current version if you specify 0.0.0
. There's probably a way to override that with config but using 0.1.0
as the base seemed like a reasonable trade off.
@@ -0,0 +1,15 @@ | |||
{ | |||
"compilerOptions": { |
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.
Is this all we want?
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.
Not sure. I used the defaults tsc --init
generates plus the extra config for the language server. It's easy enough to adjust. Most users are going to tweak their ts-config anyway. I tend to prefer minimal defaults as a consumer.
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
No description provided.