Skip to content

Use reactions for confirmation when appropriate#272

Merged
OpsBotPrime merged 7 commits into
masterfrom
reactions
Oct 3, 2024
Merged

Use reactions for confirmation when appropriate#272
OpsBotPrime merged 7 commits into
masterfrom
reactions

Conversation

@isomorpheme

@isomorpheme isomorpheme commented Sep 10, 2024

Copy link
Copy Markdown

Closes #198. This ended up being more work than you'd expect, and due to a seeming omission from GitHub's API, we can't send a reaction in every case, in which case we fall back to an old-fashioned comments. I also decided to fall back to posting a comment if the PR isn't immediately at the top of the merge queue, but maybe there's no need to be that informative.

Depends on haskell-github/github#509 to be able to use the Reactions API. I've added that as a patch to the Nix overlay; if there's contributors that want to work without Nix then it should be doable to get that patch by adding to cabal.project as well.

@isomorpheme

Copy link
Copy Markdown
Author

Example of my local hoff using reactions: https://github.com/isomorpheme/sandbox/pull/2

@rlycx rlycx left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

source code itself looks good, but could you add a test case for this (or point me to something that tests this particular feature if i overlooked it)?

Comment thread src/Github.hs
Comment thread src/GithubApi.hs Outdated
Comment thread src/Types.hs
@isomorpheme

Copy link
Copy Markdown
Author

could you add a test case for this (or point me to something that tests this particular feature if i overlooked it)?

I already added test cases here: https://github.com/channable/hoff/pull/272/files#diff-5057de5eb61e3ca52c8f206f6964e0486e9257418d5d39ab89f3cfaaf6e97839R913-R992

Very understandable if you missed that though, the diff is unfortunately large & has a lot of repetition.

@rlycx rlycx left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

neat! thank you :3

@isomorpheme

Copy link
Copy Markdown
Author

@OpsBotPrime merge

@OpsBotPrime

Copy link
Copy Markdown

Pull request approved for merge by @isomorpheme, waiting for rebase behind one pull request.

@OpsBotPrime

Copy link
Copy Markdown

Speculatively rebased as 30575d7 behind 1 other PR, waiting for CI …

@OpsBotPrime

Copy link
Copy Markdown

CI job 🟡 started.

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.

Use "reactions" for confirmation

3 participants