Skip to content

Conversation

@jorisdral
Copy link
Contributor

@phadej I hope the changes I've made make sense. I've also added a new golden test. I've not yet added a changelog entry

= ykeyValuesFilt []
[ "push" ~> branches
, "pull_request" ~> branches
]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do just

    toYaml GitHubOn {..}
        | null ghBranches
        = ylistFilt [] ["push", "pull_request","merge_queue"]
        | otherwise
        = ykeyValuesFilt []
              [ "push"         ~> branches
              , "pull_request" ~> branches
              , "merge_queue"  ~> branches
              ]

I don't see a reason to not simply always enable merge_queue.

And I especially don't like that merge_queue with branches doesn't configure branches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, that's a simpler change. I was under the impression that the merge_group trigger had no branches filter, since it's not documented anywhere

Copy link
Collaborator

@phadej phadej Aug 28, 2025

Choose a reason for hiding this comment

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

It's unclear. The GHA YAML doesn't have proper schema, and the documentation is sparse and spread out. The docs are bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems to work fine with the branches filter on well-typed/hs-bindgen#1025

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I've updated the PR

@jorisdral jorisdral force-pushed the jdral/merge-group-trigger branch from c59cfc2 to 9a61747 Compare August 28, 2025 15:30
@jorisdral jorisdral requested a review from phadej August 28, 2025 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants