danger-js
danger-js copied to clipboard
Gitlab FR: open a discussion rather than creating a note
In Gitlab merge requests, there are two types of comments:
- Note: Appears in the merge request timeline but does not require a response before merging
- Discussion: Appears as the start of a resolvable thread; requires clicking "Resolve" before merging
For Danger, a discussion is preferable IMO, since it enables the repo administrator to require that Danger's notes be marked as resolved before the code is merged, thereby increasing the likelihood that the merge request author has addressed Danger's comments.
Glancing through the code briefly, I notice that Danger already uses Discussions for inline code comments, so I think it would be trivial to use it for "main" comments as well. If this seems reasonable, I can look at a PR sometime soon.
Assuming this is like reviews in GitHub, what we have done with that in Danger ruby is allow for a subset DSL for those capabilities: https://github.com/danger/danger/issues/684#issuecomment-269467745
Depending on what you can do with it, I'd look into using about a specific DSL for this e.g.
danger.gitlab.newDiscussion((d) => {
d.message("x")
d.fail("y")
})
I probably wouldn't be up for changing the default to something which requires clicking resolve by default (not this late into the game) but it's feasible if the PR is well tested to have a config to switch to use that instead somehow.
Makes sense; thanks for the feedback. In both GitHub and Gitlab, discussions and reviews are different things, but I think I know what direction to go in now, enough to work on a PR.
Getting started with Danger and this is defiantly something I'm looking for.
Thanks
How would the example above work if importing additional Dangerfiles?
danger.gitlab.newDiscussion((d) => {
d.message("x")
d.fail("y")
})
I would think having a config option to create the whole thing as a discussion vs a note would be better, than having to use special commands in the Dangerfile.
It could work by dependency injection, e.g. you pass the discussion as a param, I would do the same for message, error etc anyway