Adds a general shape for discussions support #412
Conversation
|
OK, I think this is pretty ready to go |
elibarzilay
force-pushed
the
master
branch
3 times, most recently
from
1d39a68 to
9e931d1
Compare
| } | ||
| } | ||
|
|
||
| async function addLabel(discussion: Discussion, labelName: string, description?: string) { |
There was a problem hiding this comment.
Wouldn't it upset GH if we end up with up to thousands of labels?
But more importantly, it looks like they're project-global, so having these massive number of labels is going to make dealing with labels pretty difficult. The least that this should do is use some prefix for these things, otherwise I can post a message with a [Critical] and confuse people and code...
There was a problem hiding this comment.
Yeah, I've opted for "Pkg: " - I'd like something a bit further in the alphabet if you can thing of something better though? (emoji/non-ascii aren't counted in the sorting for labels)
There was a problem hiding this comment.
I didn't see the Pkg: change, so leaving this open. Also: I'm still worried about the first point: we have thousands of packages, and therefore we'd end up with thousands of labels, and I'm not sure how things would look like if/when we have that.
There was a problem hiding this comment.
Yeah, over a long enough timeframe we could - I doubt the long-tail of DT modules worth writing a discussion about is that long though. To my knowledge (and some googling) there is no limit to the amount of issues a repo can have, and given they all have the same prefix - removing them from the repo is a pretty trivial script if it gets out of hand
src/discussions-trigger.ts
Outdated
|
|
||
| const newLabel = await client.mutate(createMutation("createLabel" as any, { name: labelName, repositoryId: dtRepoID, color, description })) as any; | ||
| const newID = newLabel.data.label.id; | ||
| await client.mutate(createMutation<any>("addLabelsToLabelable" as any, { labelableId: discussion.node_id, labelIds: [newID] })); |
There was a problem hiding this comment.
This line should be pulled out of the if since it's the same in both sides.
There was a problem hiding this comment.
(didn't see the change, so leaving open.)
elibarzilay
force-pushed
the
discussions
branch
from
02610f0 to
05587bd
Compare
|
I did some tweaks (main one is negating the result of the verify function), and marked all of the resolved comments as such. I could do some of these too, but you said that you did them so I'm guessing that maybe you forgot to push some more changes, and if I'll fix these I'll create a conflict nightmare for you -- so left them as-is. |
* `discussions-trigger`: added some uses of `txt` to make it more readable and other minor reformatting for the gql queries * Call to `verifyIsFromGitHub` should be negated * `verifyIsFromGitHub` doesn't need to be exported
elibarzilay
force-pushed
the
discussions
branch
from
17fdbe2 to
d9c88dd
Compare
|
Alright, I'm gonna nurse this deploy |
Adds a new function route for discussions on the DT repo.
Main features:
[module-name]in the title somewhere, then it pings everyone and sets a label on the discussion for you. If you don't it asks you for info. If you don't it asks for it.Some things that this means:
Future: When you post a new request for a dt module, it pings the discord