handle pr notifications on the same thread #161
Conversation
* origin/master: Revert "DM only when user breaks a step (#160)"
* origin/master: dune: remove unused result dependency github: use ubuntu-22.04 github: remove manual apt update github: add dependabot make status notifications minimal config_docs.md: clarify why GH handle matching is needed readme.md, config_docs.md: add documentation on slack mentions
thatportugueseguy
force-pushed
the
pr-reviews-in-threads
branch
from
7f264e2 to
3843e84
Compare
|
|
||
| let get_thread_permalink ~ctx:_ (thread : State_t.slack_thread) = | ||
| Lwt.return_some | ||
| @@ Printf.sprintf "https://monorobot.slack.com/archives/%s/p%s?thread_ts=%s&cid=%s" thread.cid |
There was a problem hiding this comment.
no, this is a dummy url. The structure is correct, but the domain is not. i didn't want to use the real one. This is only used for tests, I don't think it matters.
There was a problem hiding this comment.
Can put as a comment in code please?
|
is it waiting on anything? Can you show how the messages will look like? |
|
This is how it looks: I've added these boxes and arrows to make it easier to connect the messages on the channel and the thread. The messages in boxes are independent messages, the messages with arrows are messages that are sent at the same time as the independent ones. It looks quite condensed, but we have to keep in mind that we'll have other messages in between, so these messages on the channel may/will be quite far apart. I've just noticed that I have an issue with the merge notification styles, I'm going to fix those |
|
@Khady can we merge this? |
| | Some a -> Some (sprintf "New comment by %s in <%s|%s>" sender.login comment.html_url a) | ||
| | Some a -> Some (sprintf "Commented in file <%s|%s>" comment.html_url a) |
There was a problem hiding this comment.
this change is unrelated to the PR
| | "commented" -> "commented on" | ||
| | "commented" -> "reviewed" |
There was a problem hiding this comment.
the change is unrelated to the PR
| | Opened | Ready_for_review -> | ||
| let labels_banner = show_labels labels in | ||
| ( "opened", | ||
| body | ||
| |> Option.map (fun body' -> | ||
| Option.map_default (fun labels' -> sprintf "%s\n%s" body' labels') body' labels_banner) ) |
There was a problem hiding this comment.
the change is unrelated to the PR
| let url_args = Web.make_url_args [ "channel", thread.cid; "message_ts", thread.ts ] in | ||
| match%lwt | ||
| request_token_auth ~name:"retrieve message permalink" ~ctx `GET | ||
| (sprintf "chat.getPermalink?%s" url_args) |
There was a problem hiding this comment.
| (sprintf "chat.getPermalink?%s" url_args) | |
| "chat.getPermalink" |
this is confusing, should pass url_args as another param to request_token_auth
| @@ -144,6 +146,13 @@ type auth_test_res = { | |||
| user_id: string; | |||
| } | |||
|
|
|||
| type permalink_res = { | |||
| ok: bool; | |||
There was a problem hiding this comment.
the field is unnecessary, it is handled by http_response below
This PR handles PR and PR-related github notifications on monorobot. Tackles #154 and #155.
When a PR is