-
-
Notifications
You must be signed in to change notification settings - Fork 368
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
Comments
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 |
Yea. But I would try to keep it simple like |
I don't want a CLI flag for it, it's a DSL attribute because it differs per code review service |
So instead using warn/fail etc. Custom method like danger.github.createDiscussion() ? 🤔 |
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 |
It would be great feature, in our case for Gitlab |
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? |
@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. |
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. |
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. |
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
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 👍🏻 |
Does anyone know how to make Danger reply its own comment? |
Looks like this is a way (full gist):
|
I've started to implement this feature and it looks promising. I've chosen an environment variable I'll open a PR after some more testing. |
@orta This can be closed :) |
Thanks |
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
The text was updated successfully, but these errors were encountered: