Skip to content
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

New Plugin-Structure #274

Merged
merged 16 commits into from
Nov 11, 2022
Merged

Conversation

Cahllagerfeld
Copy link
Member

@Cahllagerfeld Cahllagerfeld commented Nov 8, 2022

closes #273

  • copy new template
  • adjust directory inside commands and extensions
  • adjust package-names
  • adding new tests to the files

We'll need some documentation around that, especially the changeset thing, as the action will fail without a proper setup. @mikenikles Do you think we should adjust the Readme, or keep the docs somewhere else for now?

@changeset-bot
Copy link

changeset-bot bot commented Nov 8, 2022

🦋 Changeset detected

Latest commit: 6a208cc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
create-webstone-app Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Collaborator

@mikenikles mikenikles left a comment

Choose a reason for hiding this comment

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

A few nit picks and one outstanding question: Should we rename packages/create-webstone-app/template to templates and create a new app sibling directory next to plugin?

The app directory contains the README.md file. This way, we cleanly separate app from plugin and maybe other templates later.

@@ -0,0 +1,6 @@
{
"useTabs": true,
"singleQuote": true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure about that? We use double quotes in our code.

@@ -0,0 +1,6 @@
{
"useTabs": true,
"singleQuote": true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above, is this intended?

Copy link
Member Author

Choose a reason for hiding this comment

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

If I'm not wrong I copied the one that gets delivered when crating a new sveltekit app that has prettier configured 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 If that's the case, let's keep it so cli and web plugins have consistent formatting.

@mikenikles mikenikles mentioned this pull request Nov 10, 2022
@Cahllagerfeld Cahllagerfeld marked this pull request as ready for review November 10, 2022 16:15
@Cahllagerfeld
Copy link
Member Author

With the change in the plugin structure, we can possibly get rid of our custom gluegun implementation. Shall we do this in this PR, or in a follow-up?

In my opinion, this PR is big enough, so I'd say we should do it in a new one. In case you think we should do it with this, I'd be fine with it either. 👍

@mikenikles mikenikles merged commit b3774cb into main Nov 11, 2022
@mikenikles mikenikles deleted the cahllagerfeld/migrate-to-new-plugin-273 branch November 11, 2022 03:48
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.

Migrate to new Plugin-Structure
2 participants