Skip to content

v1 continuous integration #4

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.

Sign up for GitHub

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

Merged
braican merged 12 commits into from
Feb 23, 2023
Merged

v1 continuous integration #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
300af21
Update per packagist recs
braican Feb 16, 2023
679bbe1
Ignore on zip downloads
braican Feb 17, 2023
e8571bc
Test building assets
braican Feb 17, 2023
a356508
Remove workflows, add build
braican Feb 17, 2023
1027c28
Add husky for git hooks
braican Feb 17, 2023
3af667b
Use pre-push instead
braican Feb 17, 2023
2d322bf
Build assets
braican Feb 17, 2023
ac6b85a
Remove husky
braican Feb 17, 2023
56641f8
Try a build asset step
braican Feb 17, 2023
ec6f4de
Attach to PR branch
braican Feb 17, 2023
a752fad
Build production assets (from GitHub Actions)
braican Feb 17, 2023
1579b84
Add autoloader
braican Feb 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.babelrc export-ignore
.eslintrc export-ignore
.nvmrc export-ignore
prettier.config.js export-ignore
30 changes: 30 additions & 0 deletions .github/workflows/build-assets.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
name: Build and commit assets

on: [pull_request]

jobs:
build:
name: Build
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
repository: ${{ github.event.pull_request.head.repo.full_name }}
ref: ${{ github.event.pull_request.head.ref }}

- name: Setup node
uses: actions/setup-node@v3
with:
node-version-file: '.nvmrc'

- name: Install dependencies
run: npm ci

- name: Build
run: npm run build

- name: Add & Commit
uses: EndBug/[email protected]
with:
add: build
message: 'Build production assets (from GitHub Actions)'
13 changes: 8 additions & 5 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
# built assets
/build

# local environment
.env
error.log

# caching and debuging
# Deps
/node_modules
/vendor

vendor/**/*
Copy link

@ALJ ALJ Feb 23, 2023

Choose a reason for hiding this comment

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

@braican similar question to my gitignore/build one... why are inner /vendor folders gitignored, but not autoload, or composer/?

Copy link

@ALJ ALJ Feb 23, 2023

Choose a reason for hiding this comment

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

Prob nevermind this Q, too as there's info in the PR description

Copy link
Member Author

@braican braican Feb 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The reasoning here is similar to that of the static assets, with some additional context around the nuances of using Composer with a WordPress project.

For some setup - this plugin uses PHP namespacing and includes an autoloader that is built by Composer, which gets added to the vendor/ directory when composer install is run when working on this package.

This is where things get interesting - if this was purely a PHP package that was getting included via Composer in a typical PHP project, it would be ok for us to omit the vendor/ directory in this repo. In that case, the PHP project would composer require this package which would download this repo's code as well as any dependencies of this package to be stored alongside each other inside the parent project's vendor/ directory. Since there's an autoloader defined in this package, Composer would also set up the parent project's autoloader to include any relevant packages.

But we're talking about a WordPress application here. This package has its type set to wordpress-plugin, which we want installed in a specific directory (like a plugins/ directory) rather than the typical vendor/ directory. (See this configuration in the WSK composer.json.) In our typical WordPress case (as it's set up with WSK), when this package is downloaded via Composer the package itself is stored in the plugins/ directory but all of this package's dependencies (including the autoloader) gets downloaded to the parent application's vendor/directory. This means that the plugin doesn't have access to the dependencies or the autoloader defined in its composer.json.

The short version is that when using Composer to include this plugin in a WordPress project, the dependencies and autoloader of this package get added to a directory within the parent project where this package can't access them, which means we need to include any dependencies directly in source control. At the moment, this plugin doesn't require any external PHP packages to work so we only need to include the autoloader here in source control - if we do end up pulling in any external packages down the line, we'll also need to add an exception for those packages in the .gitignore to include them in source control as well. Note that there are a few dev-dependencies (things like linters) that will get installed when working on this plugin.

Copy link

@ALJ ALJ Feb 23, 2023

Choose a reason for hiding this comment

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

Thank you for writing that all out! @braican this makes me wonder: why publish this as a WordPress plugin and not as a plain Composer package? Are there advantages I don't see (other than making this discoverable/disable-able in the WP admin "Plugins" screen)?

I'm happy with any plan here, btw, just curious if there's context.

Copy link
Member Author

@braican braican Feb 23, 2023

Choose a reason for hiding this comment

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

It's a fair question - here's a couple of thoughts:

  • We're talking here specifically about the convention of installing this plugin by way of Composer. But that's not the only way to manage WordPress plugins; it's probably even not a very common way to manage WordPress plugins. If a dev wanted to set up this plugin without Composer (i.e. just downloading it and adding it to their WordPress site) they'd still need the autoloader and all the stuff we're including in source control here.
  • The functionality included feels like the purview of a plugin rather than a more generic PHP package - it deals with very WordPress-specific things (loading assets, registering taxonomies, etc.) and includes things like template functions, which to me feel out of place inside a vendor/ directory.
  • Installing this to the vendor/ directory would also require us to manually initialize the functionality within the plugin (kinda like how we do Timber now), as opposed to it just loading when activated as a plugin. This is maybe a smaller thing (I could see how manually initializing it with some config parameters would be beneficial to understanding some of the features), but it still feels weird.

!vendor/autoload.php
!vendor/composer
!vendor/composer/*

# caching and debuging
npm-debug.log*
yarn-debug.log*
yarn-error.log*
Expand Down
1 change: 1 addition & 0 deletions build/index.asset.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<?php return array('dependencies' => array('wp-api-fetch', 'wp-block-editor', 'wp-blocks', 'wp-components', 'wp-compose', 'wp-data', 'wp-dom-ready', 'wp-edit-post', 'wp-element', 'wp-hooks', 'wp-plugins', 'wp-rich-text'), 'version' => '42166e9cbebc9b002fd7');
4 changes: 4 additions & 0 deletions build/index.css
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions build/index.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "upstatement/ups-editorial-wp-plugin",
"type": "wordpress-plugin",
"version": "2.0.0",
"license": "MIT",
"authors": [
{
"name": "Upstatement",
Expand Down
2 changes: 1 addition & 1 deletion composer.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions vendor/autoload.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

// autoload.php @generated by Composer

require_once __DIR__ . '/composer/autoload_real.php';

return ComposerAutoloaderInite77806528b56491efceb0d413e19fe77::getLoader();
Loading