Add release-zip job and workflow

[schlessera]

Summary

release-zip

This job checks out the source, fetches Composer and npm dependencies and does a complete build.

It then uploads the produced ZIP file as an artifact with the filename having the form amp-{branch}-{short-sha}.zip.

This file was initially built within #4761, but moved into a separate PR for faster merging.

Todo

• Ensure that branch name generation works across all events.
• Use production build instead of development build Build production and development builds.
• Upload under a publicly accessible URL to the GitHub wiki as well.
• If on a PR, publish as a self-updating comment to the PR discussion.

Fixes #4766

Checklist

• My pull request is addressing an open issue (please create one otherwise).
• My code is tested and passes existing tests.
• My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

[googlebot]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[pierlon]

@googlebot I consent.

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[pierlon]

@schlessera I can't assign you as a reviewer, but feel free to do so.

[westonruter]

I recommend keeping the dev build named just amp.zip and to put the file in a dev subdirectory, with perhaps the prod version in a prod subdirectory. This will ensure that installing amp-dev.zip won't result in a directory slug of amp-dev being used.

[schlessera]

@pierlon Is the comment above with the links to the downloads not updated?

[pierlon]

@pierlon Is the comment above with the links to the downloads not updated?

It does, but the key phrase it was looking for ("Plugin builds are ready") was already in the comment, so no action was taken. I've modified the comment-on-pr job to include the commit hash in the comment (see ecb269e), and that would instead serve as the keyword to look for.

[github-actions]

Plugin builds for ca7f28f are ready 🛎️!

• Download development build
• Download production build

[schlessera]

My cook is a bit different now on the Windows system 😃:

[westonruter]

Interesting. I get the same in Gmail on MacOS Chrome:

[pierlon]

I had not noticed 😄. There is also 👨‍🍳 (man_cook) and 👩‍🍳 (woman_cook) but I did not want to make the emoji gender-specific. Removing the emoji is also an option, although I believe the comment would lose some flair 🎊.

[schlessera]

Ha, there's even italic emojis:

Also, weirdly enough, these two are displayed differently:

[westonruter]

The implementation details for the GitHub actions are beyond me, but I love the output! So as far as I know, this gets a ✅ from me.

@pierlon Anything left from your side?

@schlessera While you initially opened the PR and can't technically approve, do you approve?

[westonruter]

Oh, one thing I noticed: I did a prod build locally and compared it with a prod build and I noticed that the ZIP contains the files in the merged state with develop. In other words, there are changes in the ZIP from develop that weren't yet merged into this branch. Is that expected? I don't think it's necessarily wrong and can even be desirable since it reflects the state after merging, but I just wanted to confirm.

Aside: At some point it would be nice as well to see if the commits in a PR cleanly cherry-pick onto the most recent release branch, and if so, create builds for them as well.

[pierlon]

In other words, there are changes in the ZIP from develop that weren't yet merged into this branch. Is that expected?

No, that's not expected. I thought by default it would be using the git SHA of the commit to checkout the repository, but instead it is using the git ref of the branch, which is in this case refs/remotes/pull/4769/merge. If everyone's OK with that we can leave it as is, or make the use of the commit SHA explicit for checkout if needed.

[westonruter]

I think on Travis this is differentiated by branch builds and PR builds. Branch builds get tested in the non-merged state, whereas PR builds do. I'm not sure how that is reflected in the action configuration. But yeah, I think creating builds with the merged state is marginally better, even though there is a small possibility that someone will be confused where other changes are coming from. Better to test the continuous-integration than the isolated branch.

[pierlon]

I'm inclined to agree with you on that. It would be best to test against the latest changes from the source branch so that any regressions could be discovered before merge.

[schlessera]

@westonruter LGTM 👍