search

redhat-plumbers-in-action / differential-shellcheck

🐚 GitHub Action for running ShellCheck differentially
GNU General Public License v3.0
53 stars 9 forks source link

Add support for merge_group event #432

Closed masaru-iritani closed 2 months ago

masaru-iritani commented 2 months ago

Type of issue

Feature Request

Description

Our team uses the merge queue to serialize and validate changes on the default branch. I want to run ShellCheck when a change is added to the merge queue so that I can detect problems (e.g., introduced by a bad merge) before getting them merged to the default branch.

Describe the solution you'd like

I tried creating a change, https://github.com/masaru-iritani/differential-shellcheck/pull/1.

Things I don't understand are:

jamacku commented 2 months ago

I like your idea, but I have never used merge_group myself. Feel free to open the PR directly. It will start CI, and we can continue from there.

  • Test suite job failed due to "Error response from daemon: denied". Is there any permission I need for running it?

Not sure what exactly happened, but I guess you were missing some permissions. You can try to open PR directly, and it should work that way.

Seems like you hit this issue: https://stackoverflow.com/questions/73302997/why-do-i-get-error-response-from-daemon-denied-when-trying-to-pull-an-image-f

  • link_to_results function is supposed to return a URL for querying the code scanning results, which I cannot see in this official repository or my fork. How can I understand what kind of URL to be generated?

GitHub allows us to see scanning results only for organization members (probably because some findings might be potential security issues). I suppose you don't see them on your fork because you haven't scanned it yet. Committing to main should be enough. It will perform a base scan, and you should see all existing warnings in the Security Dashboard.

But you can check the results of Differential ShellCheck on your PR here - https://github.com/masaru-iritani/differential-shellcheck/actions/runs/10059750767?pr=1#summary-27805683474. You haven't introduced any new ShellCheck warnings.

  • Is the merge queue (able to be) enabled in this repository so that workflows in this repository can test the functionality?

We can try.

masaru-iritani commented 2 months ago

Thank you for your prompt response and kind suggestion, @jamacku!

You can try to open PR directly, and it should work that way.

As for the permission issue, I tried opening #433.

Committing to main should be enough.

Yes, after merging my pull request to the main branch of my fork, I'm able to see the code scanning results. The problem is that I cannot enable the merge queue for my fork repository because the feature only supports organization repositories, which I don't have (as an admin). I cannot tell how the link would be for a code scan result of merge_queue.

If you can enable the merge queue in this repository, that would be great. Otherwise, I need to try running my forked workflow in my organization repository, which may cause some disturbance.

jamacku commented 2 months ago

I'll try to enable merge_queue for this repo. I wanted to try it for some time anyway. :-)

jamacku commented 2 months ago

Native support for the merge_group event is available in v5.4.0.

Thank you, @masaru-iritani, for implementing this feature! I appreciate it.

masaru-iritani commented 2 months ago

@jamacku Thank you for your kind support to make this happen 🎉 I'll work on consuming the new version shortly!