DNA: Move some root-level files to the jetpack-standards repo

[kraftbj]

Changes proposed in this Pull Request:

• Move some dev environment files to a new composer package for others to also use.

Note: Does not run in production so it doesn't have to wait for Jetpack's PHP version bump.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

• Part of the DNA.

Testing instructions:

• Run composer install and verify the files deleted in this PR are added. They should not be in git.

Proposed changelog entry for your changes:

• Developer Experience: Use the new automattic/jetpack-standards package for setting up devex.

[kraftbj]

The unit testing is failing because I added composer install, which fails on PHP 5.2. Travis needs composer install because .nvmrc defines which node version is being set for the three yarn commands.

I can move the composer install to just those, but leaving it for the moment for the sake of discussion.

[roccotripaldi]

seems like a solid proposal. are there any down sides that you can think of?
are there any bits in these config files that are specific to Jetpack?

The only possibly specific bits I see are in .jshintrc
I'm not sure how the extend property behaves there, and if the overrides are jetpack specific.

Also .phpcs.xml.dist has <ruleset name="Jetpack"> - maybe rename to JetpackStandards to match with the package name?

[jetpackbot]

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: June 4, 2019.
Scheduled code freeze: May 28, 2019

Generated by 🚫 dangerJS against 7b09e55

[kraftbj]

@roccotripaldi That's a good question re .jshintrc. @simison, could you weigh in or give me a read of who to ask?

On phpcs, I'm fine with it either way. In terms of a rule name, it would be a default set of rules for the Jetpack ecosystem, so I think the generic name is fine but I don't have a strong opinion on that.

The main thrust of the PR is for infra -- does this work as a solution on having a place to put things that we want in every ecosystem plugin? -- and we can modify what goes where.

If/once we fire on @tyxla's mirroring solution, I can move the jetpack-standards repo back into the monorepo here, which will make the conversation and development of tweaking these things easier.

[zinigor]

For some reason this PR is failing on the Beta Builder with the following message:

$ gulp
[15:29:16] Requiring external module @babel/register
[15:29:20] Using gulpfile ~/tmp/jetpack/try_extract-standards-for-new-repo-6f1d49c98db0873777f28c8c53fbd8353e776369/jetpack/gulpfile.babel.js
[15:29:20] Starting 'default'...
[15:29:20] Starting 'old-styles'...
[15:29:20] Starting 'js:hint'...
[15:29:20] Starting 'php:module-headings'...
[15:29:20] Starting 'react:master'...
[15:29:20] Starting 'sass:old'...
Can't find config file: .jshintrc
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

[simison]

In a setup like this (or how I understood what this is 🙂) anything with hardcoded paths is prolly going to be problematic; eslint config for example has specific overrides, so do ignore-files.

Typical pattern for shared configs is to have versioncontrolled packages for example for eslint, and then very shallow config in consumer application that just says "use this external package as a config". See e.g. browserslist config in package.json as a good example. So instead of config sharing this looks like package bootstrapping to me, in which case you're copying a lot of code to multiple places where it could easily have one source of truth instead.

Anyway, I'm making some assumptions here. 🙂
I cannot really comment on development (or deployment?) experience without having more context; how would you use the external repo containing these files? How would PRs that need changes in these config files and files in Jetpack repo look like? E.g. if I'm adding new type of test files that need to have their own eslint rules applied?

Are we going to be developing in multiple repositories?

Thanks for the ping! I appreciate it.

[simison]

On sidenote, would be reeeeally nice to get rid of jshint soon. It's duplicating that eslint can do much better. There is a helper in Gutenberg's shared eslint config to help to do that.

[kraftbj]

Are we going to be developing in multiple repositories?
jetpack-standards will be moved into this repo once we iron out the monorepo->packagist questions. It's being dev'd in a separate repo as a temporary measure since it sounds like we'll be able to make that move with Marin's work.

[kraftbj]

@zinigor We probably need to add a composer install to the beta server's build script. (see 7b09e55 )

[kraftbj]

Typical pattern for shared configs is to have versioncontrolled packages for example for eslint, and then very shallow config in consumer application that just says "use this external package as a config". See e.g. browserslist config in package.json as a good example. So instead of config sharing this looks like package bootstrapping to me, in which case you're copying a lot of code to multiple places where it could easily have one source of truth instead.

This is good stuff. Basically, the idea is a common place for common things that shouldn't be different anywhere (like .editorconfig as a very clear example). If we want to change some of those settings over time, we would update them one place and all downstream Jetpack ecosystem plugins would enjoy the benefits of that one change.

Things like .gitignore would never make sense here for that reason. With the jshint, etc, if those are better not being standardized everywhere -- or having something standard that points to somewhere local for local modifications -- that would be a better approach.

The idea of this is not bootstrapping as much as a way to have a common source of truth for files that cannot live in the /vendor/ folder (like .editorconfig, editors will only look to the root folder).

True bootstrapping is something I'm exploring in https://github.com/automattic/jetpack-skeleton (where running composer create-project automattic/jetpack-skeleton.... would create a standard fresh bootstrapped project environment).

[simison]

Thanks for elaborating! So development wouldn't happen primarily in one repository (i.e. monorepo) and development and opening PRs would happen in several different repositories? I think I'm not entirely sure what "jetpack ecosystem" is technically. :-) Would e.g. WooCommerce use these files? How would example repository using this package look like?

I do t mean to question possible multi-repositories-setup, just trying to understand the goal. :-)

[kraftbj]

I wasn't clear. Once the monorepo->packagist idea is settled and has landed, it would all be in one repo.

Yes, multiple plugins would use it. I'm using the term "Jetpack Ecosystem" to refer to whatever plugins we want to include. For example, VaultPress is a separate plugin that we're bringing fully within the Jetpack name (e.g. "VaultPress by Jetpack"). That plugin (for sure) would use this same package to give the same development baseline as Jetpack itself (and any other plugins adopting into the system).

WooCommerce is a bit of a different case since it is well-established with separate standards in place with separate dev teams. Would be cool if they did adopt it, but I wouldn't automatically expect it. But, in a future "Jetpack Search" plugin or other smaller existing plugins or future Jetpack-minded plugins would use it.

Does that help any?

[gravityrail]

This needs a new owner who can decide whether it should be merged to master, the features packaging branch, or closed. @jeherve ?

[jeherve]

I'll take this on when I get a bit of time. 👍

[jeherve]

Closing this PR in favor of #12547 (it was easier to start from scratch since the state of the repo is quite different now).