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

Add live preview URL on Pull Request #54303

Open
wants to merge 11 commits into
base: trunk
Choose a base branch
from

Conversation

bangank36
Copy link
Contributor

@bangank36 bangank36 commented Sep 8, 2023

What?

Add PR comment with Gutenberg build status and link to gutenberg.run

Why?

Make it easier for non-tech contributors can help testing the PR as well as download the Gutenberg plugin zip file

How?

#54304

Testing Instructions

On my repo

  • Visit https://github.com/bangank36/gutenberg, since it's personal plan I removed all other Actions and only keep Build workflow to reduce queue time
  • Create a PR
  • Watch the PR comment for build status comments: in_progress at first and success when the Build ready
  • Keep pushing new commits and watch the comment update with the latest commit link and files

On Wordpress/Gutenberg

  • It has to be merged on trunk to test the workflow_run event, we may need a condition on the task file to check for a particular branch name before making it available for all future PRs

Unit testings

  • Run npm run test:unit /packages/project-management-automation/lib/tasks/pr-preview-link/test/ for prPreviewLink unit testings

Testing Instructions for Keyboard

Screenshots or screencast

image

cc: @iandunn @ockham @noisysocks

name: Pull request automation

jobs:
pull-request-automation:
runs-on: ubuntu-latest
if: ${{ github.repository == 'WordPress/gutenberg' }}
if: ${{ true || github.repository == 'WordPress/gutenberg' }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: I made this always true to test on personal repo, should be removed on trunk

Copy link
Contributor

@alexstine alexstine left a comment

Choose a reason for hiding this comment

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

This is looking really cool. I think it will help out with testing a lot.

Could you please remove the icons from the returned comment? They cause accessibility issues. Text labels are better.

@alexstine alexstine added [Type] Enhancement A suggestion for improvement. GitHub Actions Pull requests that update GitHub Actions code labels Sep 8, 2023
@annezazu annezazu self-requested a review September 12, 2023 00:10
@annezazu
Copy link
Contributor

Thanks so much for taking this on! Since this seems geared towards helping folks test without a dev environment, I wonder if we can gear the comment in that way. At a glance, folks might not realize what it's unlocking. For example, rather than saying "Gutenberg Plugin Build Status", what if it were a bit more friendly like "Contribute to this PR"? From there, the rest of the information could be presented more as a last updated with a link to Gutenberg run and some instructions around how best to use Gutenberg.run.

@bangank36 bangank36 marked this pull request as ready for review September 13, 2023 02:27
@bangank36
Copy link
Contributor Author

last updated with a link to Gutenberg run

Do you mean we may not need other rows other than the Live Preview URL?

and some instructions around how best to use Gutenberg.run.

Will this be a link to Contributor handbook for the detailed steps?

@annezazu
Copy link
Contributor

Do you mean we may not need other rows other than the Live Preview URL?

Yeah, I think we could make this more condensed and clear as the intent is to allow folks to have an easier to way to check out what's going on with the PR itself rather than to provide a status update about Gutenberg.run if that makes sense!

Will this be a link to Contributor handbook for the detailed steps?

I think this could be fairly simple. I'm not sure we have a handbook page for that but some sort of one liner around "You can use this Gutenberg.run link to quickly test and provide feedback on this PR without setting up a development environment. A Gutenberg.run site works for 24 hours. Thanks for providing feedback!"

@annezazu
Copy link
Contributor

@ryanwelcher could you help move this forward and get this approved? I think it would be a huge help for folks to give feedback on PRs.

@bangank36
Copy link
Contributor Author

Yeah, I think we could make this more condensed and clear as the intent is to allow folks to have an easier to way to check out what's going on with the PR itself rather than to provide a status update about Gutenberg.run if that makes sense!

We may still need the Built Artifact URL as suggested in this comment #27426 (comment), It helps folks who want to test the Gutenberg plugin on their own sites & data by simply clicking download link instead go to Checks tab

I think this could be fairly simple. I'm not sure we have a handbook page for that but some sort of one liner around

Then I think we can place the message inside PR template or under the build status comment like below


Contribute to this PR?

You can use this Gutenberg.run link to quickly test and provide feedback on this PR without setting up a development environment. A Gutenberg.run site works for 24 hours. Thanks for providing feedback!

Copy link
Contributor

@alexstine alexstine left a comment

Choose a reason for hiding this comment

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

@bangank36 This is looking really good. As mentioned in my previous review, please strip the icons. Thanks.

@bangank36
Copy link
Contributor Author

bangank36 commented Sep 14, 2023

please remove the icons from the returned comment? They cause accessibility issues

@alexstine is the a11y applied for github comments too? Because using icons can help identify the status quicker, as I can see they are used across the repo, eg #53360 (comment) or #54443 (comment)?
cc @annezazu

@alexstine
Copy link
Contributor

@bangank36 I've been trying to replace them but its a slow process.

@bangank36
Copy link
Contributor Author

@bangank36 I've been trying to replace them but its a slow process.

:) Sure I'll make change and update the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GitHub Actions Pull requests that update GitHub Actions code [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants