-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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(cta): add SolidJS recipe #2619
Conversation
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.
Thanks for your efforts, there is a couple of things to change (I think you copied the recipe from dev
branch instead of next
which has these fixes already).
For tests, you can add it here https://github.com/tauri-apps/tauri/blob/next/tooling/create-tauri-app/test/spawn.test.mjs#L27 and here https://github.com/tauri-apps/tauri/blob/next/.github/workflows/test-cta.yml#L8
I almost forgot, the PR is missing a changefile and please fill out the PR template instead of removing it. |
Co-authored-by: Amr Bashir <48618675+amrbashir@users.noreply.github.com>
Co-authored-by: Amr Bashir <48618675+amrbashir@users.noreply.github.com>
Co-authored-by: Amr Bashir <48618675+amrbashir@users.noreply.github.com>
My apologies @amrbashir I threw this up too quickly then rushed away to another task without reading the procedure properly. I've changed the description with the proper PR template, changefile (I looked at Svelte's changefile) and applied the changes you requested. |
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.
No need to apologize.
Just some more small changes and this PR is ready to go.
Co-authored-by: Amr Bashir <48618675+amrbashir@users.noreply.github.com>
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.
Sorry I forgot about these.
@amrbashir my apologies this PR could have gone a lot smoother. I hope things are looking much better now! |
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.
yeah the PR looks good to me now but since we are in a feature-freeze phase, it might be a while before it gets merged.
Yup no worries! Looking forward to when its merged. |
@amrbashir should I be concerned that it's not passing the eslint test? I adjusted the types to mitigate it but it's still stumbling.... |
That is actually a formatting issue, should be fixed by running |
@amrbashir the test runner didn't list any formatting errors. When I ran prettier it didn't change much except removed end of line. shrugs lol |
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
The PR fulfills these requirements:
If adding a new feature, the PR's description includes:
Other information: