Skip to content

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

Merged
merged 22 commits into from
Sep 2, 2022
Merged

feat: initial implementation #1

merged 22 commits into from
Sep 2, 2022

Conversation

tvanhens
Copy link
Contributor

@tvanhens tvanhens commented Sep 1, 2022

No description provided.

@sam-goodwin
Copy link

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

@tvanhens tvanhens changed the title Initial implementation feat: initial implementation Sep 2, 2022
@tvanhens
Copy link
Contributor Author

tvanhens commented Sep 2, 2022

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

Comment on lines +19 to +20
mv ${PACKAGE_NAME}*.tgz ${PACKED_NAME}
npm i -g ${PACKED_NAME}

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?

Copy link
Contributor Author

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 ..

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary

Copy link
Contributor Author

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 cds 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",

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

Copy link
Contributor Author

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": {

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?

Copy link
Contributor Author

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.

@tvanhens tvanhens merged commit 2fc7c9b into main Sep 2, 2022
@tvanhens tvanhens deleted the dev branch September 2, 2022 20:23
@github-actions
Copy link

github-actions bot commented Sep 4, 2022

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants