Skip to content

feat(cta): add SolidJS recipe - #2619

Merged
lucasfernog merged 19 commits into
tauri-apps:nextfrom
davedbase:next
Sep 22, 2021
Merged

lucasfernog merged 19 commits into
tauri-apps:nextfrom
davedbase:next

Conversation

@davedbase

@davedbase davedbase commented Sep 21, 2021

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Docs
  • New Binding Issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes. Issue #___
  • No

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix: #xxx[,#xxx], where "xxx" is the issue number)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

@davedbase
davedbase requested a review from a team as a code owner September 21, 2021 01:18

@amrbashir amrbashir left a comment

Copy link
Copy Markdown
Member

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

Comment thread tooling/create-tauri-app/src/recipes/solid.ts Outdated
Comment thread tooling/create-tauri-app/src/recipes/solid.ts Outdated
Comment thread tooling/create-tauri-app/src/recipes/solid.ts Outdated
@amrbashir

Copy link
Copy Markdown
Member

I almost forgot, the PR is missing a changefile and please fill out the PR template instead of removing it.

@davedbase
davedbase requested a review from a team September 21, 2021 11:31
@davedbase

Copy link
Copy Markdown
Contributor Author

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.

@amrbashir amrbashir left a comment

Copy link
Copy Markdown
Member

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.

Comment thread .github/workflows/test-cta.yml
Comment thread tooling/create-tauri-app/src/recipes/solid.ts Outdated
Co-authored-by: Amr Bashir <48618675+amrbashir@users.noreply.github.com>

@amrbashir amrbashir left a comment

Copy link
Copy Markdown
Member

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.

Comment thread tooling/create-tauri-app/src/recipes/solid.ts Outdated
Comment thread tooling/create-tauri-app/src/recipes/solid.ts Outdated
@davedbase

Copy link
Copy Markdown
Contributor Author

@amrbashir my apologies this PR could have gone a lot smoother. I hope things are looking much better now!

@amrbashir amrbashir left a comment

Copy link
Copy Markdown
Member

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.

Comment thread .github/workflows/test-cta.yml Outdated
@davedbase

Copy link
Copy Markdown
Contributor Author

Yup no worries! Looking forward to when its merged.

@lucasfernog lucasfernog changed the title WIP: Add SolidJS recipe feat(cta): add SolidJS recipe Sep 21, 2021
@davedbase

Copy link
Copy Markdown
Contributor Author

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

@amrbashir

Copy link
Copy Markdown
Member

@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 yarn format.

@davedbase

Copy link
Copy Markdown
Contributor Author

@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

@lucasfernog
lucasfernog merged commit 71ea86a into tauri-apps:next Sep 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants