GitHub workflow to build and deploy standalone plugins

[mukeshpanchal27]

Summary

Fixes #638

Relevant technical choices

For now, this PR adds a new GitHub workflow that triggers push and pull requests, which we update after the approach is approved by other members.

The main purpose is that I have to use the 10up deploy action so the PR reads the plugins.json file and sets metric values for each module that we list in that file.

I have commented out the original SVN_USERNAME and SVN_PASSWORD so we can test workflow.

Thank you, @joemcgill for all of your advice, and thank you, @costdev for all of your assistance in setting up the matrix for GHA.

Checklist

• PR has either [Focus] or Infrastructure label.
• PR has a [Type] label.
• PR has a milestone or the no milestone label.

[joemcgill]

I assume you have these here for testing, but I think we'll need to change this trigger to something different like a release, or the addition of a tag, or something. What do you think?

[costdev]

This makes sense.

Performance Lab uses:

on:
  release:
    types:
      - published

WordPress Beta Tester uses:

on:
  push:
    tags:
    - "*"

[mukeshpanchal27]

Yes @joemcgill, Once we have thoroughly tested the workflow, we must update it with a release or push tags. 

[felixarntz]

@mukeshpanchal27 Can you please add both versions of the code here so that we can proceed with the full code review? You can comment out the version which we should eventually use (on release or manual dispatch), and then add a TODO comment to get rid of the current triggers here before we merge this.

[joemcgill]

Thanks, @mukeshpanchal27, this looks like it's headed in the right direction. What are the remaining blockers, and can I help you resolve them in someway? At minimum it seems like we're waiting on the first plug-in to be available on the.org repo, but we also need to decide on how this is getting triggered. Is that something that you want to figure out, or are you looking for direction on? Any other concerns?

[mukeshpanchal27]

Thanks, @joemcgill for your review.

• The deploy process look pretty good but still we are waiting for .org repo once we get it, how do we test it? Do we need to check a different version for the plugin to be deployed?
• We need to figure out when the workflow is triggered so we don't get blocked on this.

[costdev]

If Performance Lab will have releases that shouldn't trigger the workflow, on: release: types: published doesn't seem appropriate.

Some combination of these might work:

on:
  push:
    branches:
      - 'trunk'
    paths:
      - 'modules/**'
    tags:
      - '*'

Needs some thought though.

[joemcgill]

...we are waiting for .org repo once we get it, how do we test it? Do we need to check a different version for the plugin to be deployed?

This is a tricky one. We should be confident that the script is going to work even before we have a released plugin to use this with. You can already see that the environment variables are being set correctly when you look at the failed task now, so I think we're good here. Once we have an official plugin SVN repo to test with, we can keep things as is and manually execute the workflow with this forked version of the script so we can see the whole process without pushing a new commit.

We need to figure out when the workflow is triggered so we don't get blocked on this.

I agree. For now, I still think it makes sense for us to execute these anytime a new release is published, since that is most likely the way we will want to use this task. If there are exceptions where we need to push out a manual release of either the performance lab plugin or one of the standalone plugins, we can do that manually without using this automated process. That said, I'm happy for us to come up with a better way of doing this in an automated fashion, and @costdev's suggestion is one option we should consider.

[costdev]

Do we need to check a different version for the plugin to be deployed?

From the issue:

It should then check out the individual SVN repositories for those plugins from the plugins.json file, and for each plugin replace the current trunk folder content with the newly built plugin.

From there on, it should only proceed for each plugin if the version in the plugin's main file and the plugin's readme.txt file matches and is greater than the version that was previously in those files. (The only exception is if this is the initial release, i.e. there was no version before.) If so, the next steps for each plugin are:

Copy the trunk folder to a new tags/$version folder.

SVN-add / SVN-delete any files as needed based on svn status.

Commit the new code.

☝️ This seems to provide good guidance here.

We need to figure out when the workflow is triggered so we don't get blocked on this.

From the issue:

For triggering this workflow, as a starting point we can probably set it to only run when a new Performance Lab plugin release is made, i.e. this would also trigger a new release of any standalone plugins if their versions were intentionally increased. We can always enhance this later if we e.g. also need to trigger manual releases for those plugins. Note that in the future, once the standalone plugins are no longer part of the PL plugin, the trigger will have to be changed anyway, so this is just a temporary measure.

For now, if all of the above sections are met, I think we could use this:

on:
  release:
    types: [published]
  workflow_dispatch:
    inputs:
      slug:
        type: string
        description: 'The slug of the plugin to deploy'

From there, updated plugins will be deployed when:

• Performance Lab is released, OR
• We manually trigger the deployment if the standalone update needs to deploy sooner.

inputs.slug can be checked and if not empty, can be used to only run for the specific plugin. This fine-grained control might be preferable if multiple plugins have been updated, but only one of them should be deployed before the Performance Lab release (e.g. fixing a Fatal Error)

[mukeshpanchal27]

Thanks both for your feedback.

@joemcgill We got official approval for WebP Uploads. Do we need to upload the initial version to SVN before checking GHA? What is the next step on that? Once we did all the testing, we had to decide when the workflow should trigger.

[joemcgill]

That’s great news that we have approval. As long as we have an SVN repo we should be able to test the workflow, we just don’t want to actually commit the new files to that repo until we’ve finished the whole build process.

[mukeshpanchal27]

@joemcgill The workflow is now https://github.com/WordPress/performance/actions/runs/4582789944/jobs/8093233503?pr=686 ✅ and deploy process working look good. Can you please review it. Thanks!

[felixarntz]

@mukeshpanchal27 This looks great for the most part.

In a few places, it would be great if you could explain which changes are meant to be temporary (before the merge of this PR) and which changes are meant to be actually merged. Please use code comments to make that clear from the code itself - there are a few places where I have questions about what you intend to get merged vs what is just temporary.

[felixarntz]

@mukeshpanchal27 Can you please add both versions of the code here so that we can proceed with the full code review? You can comment out the version which we should eventually use (on release or manual dispatch), and then add a TODO comment to get rid of the current triggers here before we merge this.

[felixarntz]

Why are these two lines commented out?

[mukeshpanchal27]

Because if somehow the workflow starts working, then it will not commit the code in SVN because of wrong access. I'll update it once everything is finalised. Thanks.

[joemcgill]

@felixarntz The deploy.sh code here is a direct fork of the 10up/action-wordpress-plugin-deploy action and was just added to this PR so we could test out the integration of our deployment workflow with this action without actually pushing the commit to the SVN repo. We should disregard any changes to that file in this PR since it will be removed once we have completed a dry run.

The next steps here, as I see it, are to update the .github/workflows/deploy-standalone-plugins.yml file so the workflow is triggered correctly, and so it makes use of the SVN secrets from this repo. It would be best if we could also trigger this workflow manually in order to do one last dry run before swapping out the bash script for the 10up/action-wordpress-plugin-deploy action.

[felixarntz]

Thanks for clarifying @joemcgill. In that case, some of my feedback above does not need to be actioned.

That said, two most critical things to work out from my perspective:

• Finalize the trigger logic: Let's add this in commented out code already so that we can review it. Other than when a release is published on GitHub, we should also allow triggering manually via workflow_dispatch.
• Decide how to handle assets: Right now, the way the code works, it'll use the same assets as PL. We may want to change that to point to another ASSETS_DIR to not use any assets at all for the launch. Let's discuss that separately.

[joemcgill]

Thanks @felixarntz.

Finalize the trigger logic: Let's add this in commented out code already so that we can review it. Other than when a release is published on GitHub, we should also allow triggering manually via workflow_dispatch.

In that case, I suggest we go ahead with what @costdev proposed for a manual workflow:

on:
  release:
    types: [published]
  workflow_dispatch:
    inputs:
      slug:
        type: string
        description: 'The slug of the plugin to deploy'

Since workflow_dispatch actions can only be run if the workflow is in the default branch, I don't think we need to comment that out while we review.

[10upsimon]

@mukeshpanchal27 I have no other feedback aside from what @joemcgill and @felixarntz have provided, other than a nit pick suggestion to clean up a comment, most specifically the first person speech aspect, which I think we should always avoid in comments.

[felixarntz]

Thanks @joemcgill, SGTM!

Since workflow_dispatch actions can only be run if the workflow is in the default branch, I don't think we need to comment that out while we review.

Makes sense to me. It would still be good to annotate in the PR code with comments which blocks of code are temporary vs which code should eventually be merged.

[mukeshpanchal27]

• Decide how to handle assets: Right now, the way the code works, it'll use the same assets as PL. We may want to change that to point to another ASSETS_DIR to not use any assets at all for the launch. Let's discuss that separately.

@felixarntz In the 5d4e5c0 i set ASSETS_DIR variable which find the .wordpress-org folder in module directory. Now it skip assets copy https://github.com/WordPress/performance/actions/runs/4604169975/jobs/8134862053#step:6:53 because we don't have that folder in module directory. In future we can manage the assets from that folder.

[felixarntz]

@mukeshpanchal27 Thank you for the updates, this looks great now.

Consider the code approved from my side, of course pending the final changes following the TODO comments.

We also still have to make a decision outside of this PR on the assets to use, but conceptually setting ASSETS_DIR to a module-specific assets directory certainly makes more sense than pointing to the same directory that the PL plugin uses.

[mukeshpanchal27]

@joemcgill @felixarntz In 748df3b commit, I update PR so it will set the matrix for the release workflow conditional base.

Note: For manual triggers, we don't use plugin names, so I have not included them.

[10upsimon]

@mukeshpanchal27 this is looking good to me now.

[costdev]

Think this looks good to me too @mukeshpanchal27! Thanks! 👍

[joemcgill]

This is a clever way to use the inputs of the manual workflow. Nice work @mukeshpanchal27. I say we go with this for our dry run, and we can decide if we want to improve the way the inputs are working before finalizing the whole feature.

[felixarntz]

@mukeshpanchal27 One last question here, but this is looking basically ready to merge.

cc @joemcgill

[felixarntz]

One last thing here before we merge this pull request (thanks @joemcgill).