-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[core] Add pr-html-generator #2525
Conversation
The check fails for this bridge since I didn't change any bridges btw. That doesnt happen that often on normal PRs and even if, it would be hard to tell which bridge to test if someone changes something in a core file :) |
What do you think of putting the link through /~https://github.com/htmlpreview/htmlpreview.github.com so that the html is rendered? |
@yamanq ouuuuuh, thats exactly what I was looking for for 20 minutes :D Thanks, I'll see what I can do since its a github action thats doing it. |
files: | | ||
./results/*.html | ||
- name: Upload results to PR | ||
uses: gavv/pull-request-artifacts@v1.0.0 |
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'm not sure how actions work exactly, but I forked the one used in this PR, changed a line, and tagged it. Would this work then?
uses: gavv/pull-request-artifacts@v1.0.0 | |
uses: yamanq/pull-request-artifacts@v1.1.0 |
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 think you changed it in the wrong line. Try to build the string in line 156
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.
Thanks, fixed and tested this time. See test and required changes.
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.
Very cool, thanks! I will adap the action. I currently have another issue (as you can see from the actions) that there seems to be a permission issue somewhere thats not happening in my own branch.
Looking at the existing workflow, it already uses pull_request_target, so ignore my previous comment. This article might be more helpful. |
Yeah, I've been trying furiously as you can see :D I've also seen that article, but I haven't tried the double-set yet (basically reacting on lint workflows for example). Either I dont understand how pull_request_target should works or it's not working as intended (it has not fired once in this whole PR). |
Is it possible that the action must exist in the target branch first? |
I dont think so. The same action (just with on: pull_request) works fine. You can see the runs and failed runs of it in the repo actions history. It's running... just not with pull_request_target... and I have no idea why... |
Oh wait... there could be something to it... Let me try with my other repo |
Okay.... that was actually it. And in my shame, it actually makes sense. You can add a workflow.yml in a PR and it will run it no problem (if you use on: pull_request). It will NOT do it if you use pull_request_target because you could just go "echo secrets.*" or something like that in a new PR on any repo So.. the solution is: accept that the workflow works in my repo and then just merge ^^ |
Okay, I will merge this commit then. Worst thing that can happen is that a few tests fail for new PRs. Changes can be done in further PRs. Thanks a whole lot @yamanq ! |
Greetings,
this is a new quality checking workflow.
This will automatically generate the html output of any changed bridges of a PR, for any context of the bridge.
You can see it in action here (its a dummy repo of mine, hardwired to a specific PR. the code in this PR uses the actual PR number that it's running for).
@em92 this would replace the manual process/script for testing. This will also check/indicate if all example/default values are correctly defined. It tests all contexts so it's basically an automatic full-test of this specific bridge. Thoughts?
Additionally: I wrote this in python but it's probably just as doable in php. If you want to keep even the helper/quality scripts php only, I would still advocate for merging and then someone else can do the php version :)