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

Workflows: Sync assets to plugin repo upon change in trunk #68052

Merged
merged 13 commits into from
Dec 19, 2024

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Dec 17, 2024

Fixes #67835.

What?

Sync the assets/ directory to the plugin svn repo whenever a file in that directory is changed as part of a commit to trunk

Why?

To sync the plugin repo's assets for the Gutenberg plugin with the latest changes from git.

How?

By checking out the assets/ directory from git, and using it to replace the contents of the assets in the Gutenberg plugin's svn directory.

Testing Instructions

I have a test SVN repo that I had already set up to include the assets/ directory from the GB plugin repo, plus a test.txt file in that directory.

I tested this workflow by adding a commit to set the plugin repo URL to that test svn repo, and force-pushing the git branch to my personal Gutenberg fork's trunk. The workflow run passed, you can see it here. (It removed the test.txt file, as it's not present in git).

The commit message recorded in my test SVN repo is Sync assets for commit 97de59b764c9017242d1da44a034015fc262fb80.

@ockham ockham added [Type] Enhancement A suggestion for improvement. [Type] Build Tooling Issues or PRs related to build tooling Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts labels Dec 17, 2024
@ockham ockham requested review from bph and sirreal December 17, 2024 13:05
@ockham ockham self-assigned this Dec 17, 2024
@sirreal
Copy link
Member

sirreal commented Dec 17, 2024

Should the entire assets dir be synced? It would have to be updated from trunk first. It seems like either the assets should be managed from this repo or not, it seems kind of strange to manage some assets, especially since we have an assets directory now.

If we're only going to sync blueprints, then perhaps the directory structure should be rethought in this repo.

@sirreal
Copy link
Member

sirreal commented Dec 17, 2024

After briefly reflecting on it myself, I do think the whole assets directory should be managed from this repo. Ultimately, the plugin itself is the most important asset and that's managed from here.

@ockham
Copy link
Contributor Author

ockham commented Dec 17, 2024

Yeah, that's a fair point. This means we'll have to add the existing assets to the GH repo. We can do that as part of this PR.

@sirreal sirreal removed the [Type] Enhancement A suggestion for improvement. label Dec 17, 2024
Copy link

github-actions bot commented Dec 17, 2024

Flaky tests detected in 10aaba2.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12394362673
📝 Reported issues:

@ockham ockham added the [Type] Enhancement A suggestion for improvement. label Dec 17, 2024
@bph
Copy link
Contributor

bph commented Dec 17, 2024

Thank you @ockham @sirreal and @mcsf for taking care this. If understand it all correctly, to update the blueprint.json in the future one would only need to submit a PR here and It automatically updates it on the Plugin directory when the next GB version is released.

@ockham
Copy link
Contributor Author

ockham commented Dec 17, 2024

Thank you @ockham @sirreal and @mcsf for taking care this. If understand it all correctly, to update the blueprint.json in the future one would only need to submit a PR here and It automatically updates it on the Plugin directory when the next GB version is released.

Yeah, that's correct.

Now that you mention it, we can also change that behavior, to decouple it from plugin uploads.
Think we should run it on a different trigger? We could e.g. sync it whenever trunk has a commit merged that changes the file (or anything in assets/, maybe) 🤔

@sirreal
Copy link
Member

sirreal commented Dec 17, 2024

It makes sense to sync when the assets change, in my opinion. There's no reason for this to be coupled to a plugin release.

@mcsf
Copy link
Contributor

mcsf commented Dec 17, 2024

It makes sense to sync when the assets change, in my opinion. There's no reason for this to be coupled to a plugin release.

I tend to agree, but loose opinion.

ockham and others added 2 commits December 17, 2024 18:42
Co-authored-by: sirreal <jonsurrell@git.wordpress.org>
Co-authored-by: Miguel Fonseca <150562+mcsf@users.noreply.github.com>
svn st | awk '/^?/ {print $2}' | xargs -r svn add
svn st | awk '/^!/ {print $2}' | xargs -r svn rm
svn commit "assets" \
-m "Sync assets for version $VERSION" \
Copy link
Member

Choose a reason for hiding this comment

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

I think we can swap the version for the commit sha.

name: Sync assets
runs-on: ubuntu-latest
environment: wp.org plugin
if: ${{ github.event.release.assets[0] }}
Copy link
Member

Choose a reason for hiding this comment

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

I think we want to filter to:

  • run only on commits
  • only to trunk branch
  • only when there are changes in assets/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll probably move the entire job to a whole new workflow file, with the trigger set accordingly 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

.github/workflows/upload-release-to-plugin-repo.yml Outdated Show resolved Hide resolved
@ockham
Copy link
Contributor Author

ockham commented Dec 18, 2024

I'm currently testing this on my personal fork, and will be pushing fixes based on that testing shortly.

@sirreal
Copy link
Member

sirreal commented Dec 18, 2024

One thing to consider, if this is landed with the new assets I'd expect it to immediately run when it's merged.

@ockham
Copy link
Contributor Author

ockham commented Dec 18, 2024

One thing to consider, if this is landed with the new assets I'd expect it to immediately run when it's merged.

You're correct @sirreal 😄 That's what happened on my personal fork 👍

@ockham
Copy link
Contributor Author

ockham commented Dec 18, 2024

This should be ready for review. I've documented how I tested this in the PR description's Testing Instructions.

@ockham ockham changed the title Workflows: Sync blueprint when uploading GB to plugin repo Workflows: Sync assets when uploading GB to plugin repo Dec 18, 2024
@ockham ockham marked this pull request as ready for review December 18, 2024 14:06
@ockham ockham requested a review from desrosj as a code owner December 18, 2024 14:06
Copy link

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ockham <bernhard-reiter@git.wordpress.org>
Co-authored-by: sirreal <jonsurrell@git.wordpress.org>
Co-authored-by: mcsf <mcsf@git.wordpress.org>
Co-authored-by: bph <bph@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@ockham ockham changed the title Workflows: Sync assets when uploading GB to plugin repo Workflows: Sync assets to plugin repo upon change in trunk Dec 18, 2024
@ockham
Copy link
Contributor Author

ockham commented Dec 18, 2024

Note that this since this new workflow uses SVN credentials, the workflow needs approval by a member of the gutenberg-core team before it can run (similar to how the plugin zip upload workflow does). We should document that somewhere; I'm not sure where. Maybe @bph can advise 😊

@bph
Copy link
Contributor

bph commented Dec 18, 2024

Maybe best to add a section to the Release process > Synchronizing the Gutenberg plugin @ockham What do you think?

@ockham
Copy link
Contributor Author

ockham commented Dec 18, 2024

Maybe best to add a section to the Release process > Synchronizing the Gutenberg plugin @ockham What do you think?

Hmm, not 100% sure TBH, since those docs are very much focused on individual GB releases and the publishing of npm packages, which is somewhat coupled to the latter. However, the workflow that this PR introduces will be pretty much independent from those existing workflows, and will run any time a file in assets/ is modified.

Maybe it's fine to document it there, but I'm a bit concerned about visibility (those docs are quite verbose already) -- will people go to look there if they change an asset?

Come to think of it, we could maybe add a README.md to assets/ in GitHub (that we don't sync to svn) with some info 🤔

Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

This seems sufficient, certainly as a first step. Thank you!

@ockham ockham removed the [Type] Enhancement A suggestion for improvement. label Dec 19, 2024
@ockham
Copy link
Contributor Author

ockham commented Dec 19, 2024

Thank you @sirreal!

I'll merge this, and will file a follow-up to add a README (as mentioned here) (plus the necessary changes to exclude the README itself from being synced).

BTW as you said, merging this PR will trigger the new workflow. There's a chance that it'll fail, as the assets we're including are identical to the ones that are already in svn, so the commit won't include any changes (and I'm not sure what svn does in a case like that). That shouldn't be a problem though -- any future changes to the assets will mean that they're actually different from the ones in svn, and will trigger the sync, as desired.

@ockham ockham removed the [Type] Build Tooling Issues or PRs related to build tooling label Dec 19, 2024
@ockham ockham merged commit 5e362f5 into trunk Dec 19, 2024
66 of 70 checks passed
@ockham ockham deleted the add/blueprint-syncing-to-upload-workflow branch December 19, 2024 09:15
@github-actions github-actions bot added this to the Gutenberg 20.0 milestone Dec 19, 2024
@ockham
Copy link
Contributor Author

ockham commented Dec 19, 2024

Here's the workflow run.

It doesn't seem to have committed anything to svn; it also didn't fail. Should be fine.

@sirreal
Copy link
Member

sirreal commented Dec 19, 2024

Come to think of it, we could maybe add a README.md to assets/ in GitHub (that we don't sync to svn) with some info 🤔

Actually, it would be good to add and sync that README file to the plugin repo so it's clear everywhere that the assets directory is managed via this repository and direct changes to SVN will be overwritten.

I asked if there's any downside to adding other files to the assets directory here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add /assets folder to plugin build process
4 participants