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

Make only-new-issues work with Merge Queues, not just Pull Requests #956

Closed
2 tasks done
andrewedstrom opened this issue Jan 24, 2024 · 9 comments · Fixed by #1029
Closed
2 tasks done

Make only-new-issues work with Merge Queues, not just Pull Requests #956

andrewedstrom opened this issue Jan 24, 2024 · 9 comments · Fixed by #1029
Labels
enhancement New feature or request

Comments

@andrewedstrom
Copy link

andrewedstrom commented Jan 24, 2024

Welcome

  • Yes, I understand that the GitHub action repository is not the repository of golangci-lint itself.
  • Yes, I've searched similar issues on GitHub and didn't find any.

Your feature request related to a problem? Please describe.

only-new-issues is fantastic, but it currently only works when the GitHub event_name is pull_request. Please add support for the merge_group event name, so that this feature can work with merge queues as well.

Merge queues are a powerful feature of GitHub for teams that merge many pull requests per day. It lets you queue up multiple pull requests to be merged in a row. Adding a PR to the queue triggers a new run of the status checks, and once the required status checks pass, that PR is merged.

Describe the solution you'd like.

We just need only-new-issues to apply when the GitHub event is merge_group. Insrc/run.ts we would change this:

const ctx = github.context
  if (ctx.eventName !== `pull_request`) {
    core.info(`Not fetching patch for showing only new issues because it's not a pull request context: event name is ${ctx.eventName}`)
    return ``
  }

to this:

const ctx = github.context;
if (ctx.eventName !== `pull_request` && ctx.eventName !== `merge_group`) {
  core.info(`Not fetching patch for showing only new issues because it's not a pull request or merge group context: event name is ${ctx.eventName}`);
  return ``;
}

(I'm assuming there would need to be some additional changes to figure out the patch, but nothing too bad)

Describe alternatives you've considered.

My main alternatives are

  1. Fix all linter errors in my project at once (hard)
  2. Disable the linter as a required check.

Additional context.

No response

@ldez ldez added the enhancement New feature or request label Feb 8, 2024
@Sortren

This comment was marked as off-topic.

@andrewedstrom

This comment was marked as off-topic.

@strantalis
Copy link

Our org is running into a similar issue as well. I probably missed something that don't understand about merge groups but this is what I changed. It seems to work but not sure if thats exactly what's needed to support merge groups.

strantalis@f4e979f

@flimzy
Copy link

flimzy commented Apr 10, 2024

@strantalis Did/can you createa PR with those changes, to hopefully get a full review?

@strantalis
Copy link

@flimzy I don't have a pr open. I will open one to hopefully get a conversation started.

@ldez
Copy link
Member

ldez commented Apr 26, 2024

I create a test repo: I can never trigger the merge_group event.

https://github.com/golangci/friendly-lamp/blob/main/.github/workflows/merge-group.yml

Can anyone provide a working example of merge_group/merge queue?
Can anyone drive me to a working configuration?

EDIT: I found how to trigger the event.

@ldez
Copy link
Member

ldez commented Apr 27, 2024

Currently, only-new-issues only works for PR (events pull_request and pull_request_target) because it uses the GitHub API to get the diff from a PR (and not because of the if on the event name).

The addition of ctx.eventName !== `merge_group` will not work because merge_group event doesn't provide information about a PR.
The PR number can be found by parsing a ref name but this is not the right way because the diff will come from PR information and not from the rebased/merged/squashed branch commits of the queue.

The support of merge_group with only-new-issues requires writing a dedicated implementation, completely different than the PR support, and will require setting fetch-depth to 0 (because a git diff should be done).


Currently, as a workaround to use merge_group, you should:

  • not use only-new-issues but args
  • use 2 conditional steps for golangci-lint
  • set fetch-depth to 0
name: golangci-lint
on:
  pull_request:
  merge_group:

jobs:
  golangci-lint:
    name: lint
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0

      - uses: actions/setup-go@v5
        with:
          go-version: stable
      
      - name: golangci-lint (PR)
        if: ${{ github.event_name == 'pull_request' }}
        uses: golangci/golangci-lint-action@v5
        with:
          version: v1.57
          only-new-issues: true

      - name: golangci-lint (queue)
        if: ${{ github.event_name == 'merge_group' }}
        uses: golangci/golangci-lint-action@v5
        with:
          version: v1.57
          args: --new-from-rev=${{ github.event.merge_group.base_sha }}

@ldez
Copy link
Member

ldez commented Apr 29, 2024

FYI, since v5.1.0, the events pull_request, pull_request_target, push, and merge_group are supported by default (no workaround needed).

Note: for merge_group, you still need to set fetch-depth to 0.


Examples

For pull requests:

name: golangci-lint
on:
  pull_request:

jobs:
  golangci-lint:
    name: lint
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-go@v5
        with:
          go-version: stable
      
      - name: golangci-lint
        uses: golangci/golangci-lint-action@v5
        with:
          version: v1.57
          only-new-issues: true

For push:

name: golangci-lint
on:
  push:

jobs:
  golangci-lint:
    name: lint
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-go@v5
        with:
          go-version: stable
      
      - name: golangci-lint
        uses: golangci/golangci-lint-action@v5
        with:
          version: v1.57
          only-new-issues: true

For pull requests, and push:

name: golangci-lint
on:
  push:
  pull_request:

jobs:
  golangci-lint:
    name: lint
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-go@v5
        with:
          go-version: stable
      
      - name: golangci-lint
        uses: golangci/golangci-lint-action@v5
        with:
          version: v1.57
          only-new-issues: true

For pull requests and merge queues:

name: golangci-lint
on:
  pull_request:
  merge_group:

jobs:
  golangci-lint:
    name: lint
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0

      - uses: actions/setup-go@v5
        with:
          go-version: stable
      
      - name: golangci-lint
        uses: golangci/golangci-lint-action@v5
        with:
          version: v1.57
          only-new-issues: true

For pull requests, merge queues, and push:

name: golangci-lint
on:
  push:
  pull_request:
  merge_group:

jobs:
  golangci-lint:
    name: lint
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0

      - uses: actions/setup-go@v5
        with:
          go-version: stable
      
      - name: golangci-lint
        uses: golangci/golangci-lint-action@v5
        with:
          version: v1.57
          only-new-issues: true

@strantalis
Copy link

@ldez Workflows seem to be working. Just wanted to say thanks for the help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
5 participants