-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Post a Playground preview link on every PR #5526
Conversation
This action adds a comment, so perhaps there's something to glean from that to leverage here? https://github.com/WordPress/wordpress-develop/blob/trunk/.github/workflows/welcome-new-contributors.yml cc: @desrosj |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should have a separate workflow for this. I think that this should be a step within the Build WordPress workflow after the artifact is built and uploaded.
Thanks you @desrosj @swissspidy! I just addressed the feedback on this PR, although we won't able to test it without merging. GitHub actions docs says:
|
This workflow now uses a different trigger ( |
@adamziel you can test this by opening a PR on your own fork instead this repository. This way we can refine it before merging. We can also try using a different trigger with a custom access token if we want to merge the workflow files. |
What was the reason pull request wouldn't work? I think I'm missing that. |
@desrosj correct me if I‘m wrong, but permissions are different for that trigger. See #5526 (comment) |
Ah, sorry I missed that because it was collapsed in the resolved discussion. We should be able to use |
And why can't we test this by opening a PR against the fork? That's how I successfully tested this in the past. This comment is created by the GitHub Action: swissspidy#33 |
…dpress-develop into preview-comment-on-every-pr
I'm on a meetup this week and AFK the next one – feel absolutely free to take over here |
Because this is required for building a zip artifact, it must run on all pull requests.
@desrosj did you manage to get it to work in your fork? |
I did, but I have a bit of cleanup to do. I plan to circle back to this later this week after 6.4 is out! |
OK, I think I've cleaned up my work and have it ready to go. You can see a test comment in desrosj#141. I chatted with @swissspidy, and thought about trying this way forward. We are already building WordPress in the workflow that tests the build process. So it makes sense to just perform the zipping and uploading of the artifact in that workflow instead of having a completely different one. That doesn't solve the issue with requiring To solve this, I've consolidated the welcome comment workflow with the logic that comments with Playground details. It will run when a PR is opened on I don't love that it will run every time a PR is updated, but I'm not sure any way around that. |
@desrosj I just reviewed and this looks good to me, thank you so much for taking the lead here! It indeed a pity this needs to run on every PR update, but I don't think it's a big deal. I like how you consolidated that with the new contributor comment 🎉 Let's ship it! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's give it a go!
Merged in https://core.trac.wordpress.org/changeset/57124. However, it seems to be having issues dispatching the workflow: https://github.com/WordPress/wordpress-develop/actions/runs/6907809373/job/18797346079#step:2:82. |
@desrosj weird! I see the proper
|
So workflow_dispatch doesn't come with the same note about permissions as
What if we just lean on |
This was merged in #5737 |
Very simple GitHub workflow to comment on every
trunk
PR with a link to a Playground-based preview.Other than access, it seems to be fine. The last thing to solve is the 403 error:
https://github.com/WordPress/wordpress-develop/actions/runs/6572036562/job/17852349020?pr=5526
cc @jeffpaul
Trac Ticket: https://core.trac.wordpress.org/ticket/59416.