Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Use Thread instead of comment #1138

Closed
shyim opened this issue May 11, 2021 · 18 comments
Closed

[Feature Request] Use Thread instead of comment #1138

shyim opened this issue May 11, 2021 · 18 comments

Comments

@shyim
Copy link
Contributor

shyim commented May 11, 2021

Describe the bug
Sometimes you want that people resolve the danger comment. Example on a fail message.

I would like a new command-line option that forces to create a thread instead of a comment.

What do you think about that @orta . I would also provide a PR

@shyim shyim added the bug label May 11, 2021
@orta
Copy link
Member

orta commented May 11, 2021

Assuming you mean reviews for GitHub? I think this pattern from Danger Ruby can work: #1090 (comment)

Personally, I'm not much a fan of the pattern because it means a lot of notifications but I can see folks wanting it

@shyim
Copy link
Contributor Author

shyim commented May 11, 2021

Yea. But I would try to keep it simple like danger ci --use-thread

@orta
Copy link
Member

orta commented May 11, 2021

I don't want a CLI flag for it, it's a DSL attribute because it differs per code review service

@shyim
Copy link
Contributor Author

shyim commented May 11, 2021

So instead using warn/fail etc. Custom method like danger.github.createDiscussion() ? 🤔

@orta
Copy link
Member

orta commented May 11, 2021

Hrm, maybe it should be a CLI param, we do that for checks. That would make life easy for downstream languages like swift/kotlin/python etc and doesn't change the JSON contract either.

I'm not sure on the name, because it's a 'review' in GitHub, a 'discussion' in GitLab and I'm sure bitbucket/azure have their own thing too. For now, let's say --use-thread but I might change that opinion in the future 👍🏻

@punk-dev-robot
Copy link

It would be great feature, in our case for Gitlab

@fbartho
Copy link
Member

fbartho commented Apr 8, 2022

I think this feature is now implemented by #1176 for Github (there's no Commandline option, that's just how DangerJS now works). Do you want to do the same for Gitlab @kuba-gaj?

@dan-trewin
Copy link

@fbartho I would very much be in favor of danger comments being threads (rather than notes) for gitlab.

The reason being: In gitlab you can require all threads be resolved before merging an MR is allowed.

My team's usecase is when danger reports potentially dangerous errors. This should block the MR by default (a thread would effectively do this), but if the author/contributors are sure that the changes are safe, they can resolve the danger comment. Effectively, this is them acknowledging the potentially dangerous errors.

Honestly, it would be nice if notes/threads were both an option, but since danger with github defaults to threads, I would think gitlab should work the same.

@fbartho
Copy link
Member

fbartho commented Apr 12, 2022

I have no stake in the gitlab platform, so I’m not going to say anything other that that your point does seem reasonable! Motivated contributors should feel free to make a clean PR to do this!

That said: each repository vendor has slightly different concepts, so I wouldn’t use GitHub to decide what to do on Gitlab. We want these platforms to feel natural for devs that only use a given provider.

@erNail
Copy link

erNail commented Jul 6, 2022

My team would also be very interested in this. We want our team members to acknowledge the result of danger.js, and Threads would be perfect for this.

@olte-mms

This comment was marked as duplicate.

@OtaconnSHGR

This comment was marked as duplicate.

@orta
Copy link
Member

orta commented Aug 29, 2022

This isn't the sort of OSS project where you can request features and the maintainers will build it for you, so +1s are not useful here. If you'd like to see it, I'd recommend you start looking at implementing it 👍🏻

@beshur
Copy link

beshur commented Aug 31, 2022

Does anyone know how to make Danger reply its own comment?
This would create a thread in gitlab

@beshur
Copy link

beshur commented Sep 1, 2022

Looks like this is a way (full gist):

danger.gitlab.api.MergeRequestDiscussions.create(
          danger.gitlab.metadata.repoSlug,
          danger.gitlab.metadata.pullRequestID,
          "The new thread",
);

@uncaught
Copy link
Contributor

I've started to implement this feature and it looks promising.

I've chosen an environment variable DANGER_GITLAB_USE_THREADS to enable it, so both are still supported and threads are opt-in.

I'll open a PR after some more testing.

@uncaught
Copy link
Contributor

@orta This can be closed :)

@orta
Copy link
Member

orta commented Nov 16, 2022

Thanks

@orta orta closed this as completed Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants