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

Stub basic post type and API endpoint #6

Merged
merged 1 commit into from
Oct 29, 2020
Merged

Stub basic post type and API endpoint #6

merged 1 commit into from
Oct 29, 2020

Conversation

iandunn
Copy link
Member

@iandunn iandunn commented Oct 28, 2020

@iandunn iandunn self-assigned this Oct 28, 2020
@iandunn iandunn force-pushed the stub-pattern-cpt branch 2 times, most recently from 7dacd55 to 9b97f44 Compare October 28, 2020 15:11
@iandunn iandunn added the [Component] Pattern Directory The backend of the pattern directory: submission, management, etc label Oct 28, 2020
Copy link
Contributor

@ryelle ryelle left a comment

Choose a reason for hiding this comment

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

CPT looks good (once I locally fixed the console warning with the keys) 👍

Since this isn't using wp-scripts, it's not running the linter, and there are lint issues in the JS. You should be able to copy the pattern-creator package.json entirely, then add this project as a workspace here, and it'll automatically work. Well, the JS file will need to move to src, or you can pass --entry=./javascript/pattern-post-type.js to wp-scripts, it'll still build to /build. This can happen in this PR or a follow up.

Comment on lines +2 to +4
* There's no need for webpack etc, since we can assume wp-admin users will have modern browsers.
*
* JSX is the only thing that'd be nice to have, but it's not worth the tooling costs for just a few fields.
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're already setting up the workspaces with a shared wp-scripts build process, I think it would be a better dev-experience to have this use it - we can let the build step handle pulling out dependencies, and will save us from needing to remember that one place uses a different syntax than others.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can see the appeal of everything using the same tooling, but it still feels to me like this doesn't need any tooling, and that the devex is better without me.

The existing tooling might might it easier to set this up, but it could still be a pain. The biggest cost IMO is using the tooling, though. It's slow, it's breaks often and is painful to fix, etc.

This is already working, too, so I don't see any benefit from redoing it now. If we want to add more fields or components later, then I can see that making the plugin complex enough where JSX would be worth the costs.

iandunn added a commit that referenced this pull request Oct 29, 2020
The Directory plugin has it's own version, and the two will be reconciled later on.

See #5
See #6
@iandunn
Copy link
Member Author

iandunn commented Oct 29, 2020

the console warning with the keys

What was that?

@iandunn
Copy link
Member Author

iandunn commented Oct 29, 2020

Oh, you were getting that typical warnings about missing keys. Huh, that's weird that I didn't. Oh well 🤷🏻‍♂️

@iandunn
Copy link
Member Author

iandunn commented Oct 29, 2020

I'm gonna merge this since I'm leaving on AFK soon, but feel free to open an issue about JSX, or just make the change if you feel strongly.

@iandunn iandunn merged commit ecf2c2a into trunk Oct 29, 2020
@iandunn iandunn deleted the stub-pattern-cpt branch October 29, 2020 14:23
@iandunn iandunn mentioned this pull request Nov 9, 2020
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Pattern Directory The backend of the pattern directory: submission, management, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stub pattern post type, meta, taxonomies
2 participants