-
Notifications
You must be signed in to change notification settings - Fork 8
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: use cookiecutter #4
Conversation
@@ -35,4 +35,4 @@ jobs: | |||
- name: Tests | |||
run: npm run test:ci | |||
- name: Codecov test coverage | |||
run: bash scripts/coverage.sh "${{ matrix.os }}" "${{ matrix.node-version }}" | |||
run: npm run test:report:coverage "${{ matrix.os }}" "${{ matrix.node-version }}" |
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.
Good call 👍
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.
Going to update this as an additional step is required for Windows machines:
https://github.com/netlify/cli/runs/1373808824?check_suite_focus=true#step:8:8
netlify/cli@efc2143#diff-7829468e86c1cc5d5133195b5cb48e1ff6c75e3e9203777f6b2e379d9e4882b3R40
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 should be fixed by [email protected]
(see https://github.com/netlify/cli/pull/1542/files)
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.
Resolved by ff887c8
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.
Could we use the new test coverage setup?
See netlify/cli#1542
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.
Sure, going to update this soon
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.
Done in 6f25f9f
This is a good idea @erezrokah! I have been re-inventing the wheel in |
I'm not familiar with
Yes, I completely missed that when I used |
78c5941
to
ebc3d11
Compare
Would a Node.js based templating generator be better in this case? So people can just |
If we can find a good one yes. |
What do you think of |
Seems like it works best for specific files (e.g. we would need a template for All we need is a utility that wraps strings replacements in a CLI. |
Yes, that's definitely a simpler approach, better for our use case. |
Co-authored-by: ehmicky <[email protected]>
Co-authored-by: ehmicky <[email protected]>
5f36e1d
to
bea4b0f
Compare
Should we add an issue to this in |
|
You can test this branch by running
cookiecutter gh:netlify/node-template --checkout feat/use_cookiecutter
once you install cookiecutter.We can further automate using https://cli.github.com/manual/gh_repo_create
TODO: