-
Notifications
You must be signed in to change notification settings - Fork 21
Reactor/Linear: Don't expand more than 3 mentions in one message #280
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
Reactor/Linear: Don't expand more than 3 mentions in one message #280
Conversation
|
Hmmm, I don't think I like this... I noticed the example where you hit a problem, you did roughly: And then synergy basically repeated your post with I don't think that behaviour is useful, because synergy is expanding what's already a URL that includes the issue ID and the description of the issue into a different formatted version of the same information. I think instead synergy should not expand those kinds of urls. That is, if it's already well formed and contains the ticket description, don't bother expanding it... That way, this kind of post still gets replied to with useful info: but we don't double up on I would also have synergy continue to expand |
|
@wolfsage I'm not sure what you're proposing to be the exact criteria for when to expand and when not to expand, and I'm not sure it would be useful.
How would you code this? |
|
I think @wolfsage was suggesting that we'd expand "without limit" if the message contained only |
Err, no, I think we just don't have a limit, and only expand IDs / URLs that don't include a description. If I post: The first two would be expanded, the third would not. |
|
I don't have strong feelings here, but I went and looked at some examples of this stuff in a scrum channel and I think I agree with Matthew's preference. If you like, I will do the work to make it work this way, @lerlacher-fm Shall I, or are you heart-set on the implemented behavior? |
|
I'm not quite satisfied.
IMO that is weird behavior that would be surprising to most people; worse, it wouldn't work for my specific bug-bear because sometimes I will just post "ABC-123" rather than the full link. Would you be happy with the behavior of "if there's more than 3 links, then instead of unfurling, post a <would you like me to unfurl these? yes/no> dialog"? |
|
What if we had a per-user preference so your things were limited or not expanded but mine were? |
|
( You could do the "expand yes / no" if you like but I wouldn't want that for my stuff I'd just want it to expand ) |
3676b61 to
65cb1c3
Compare
rjbs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two easy fixes. Thanks for making the changes.
65cb1c3 to
838d9ac
Compare
rjbs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. One inline suggestion made.
lib/Synergy/Reactor/Linear.pm
Outdated
| # if there's more than 3 issues to unroll that's probably spam | ||
| my $max_matches = | ||
| $self->get_user_preference($event->from_user, 'expando-limit'); | ||
| return unless (@matches <= $max_matches); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| return unless (@matches <= $max_matches); | |
| return unless @matches <= $max_matches; |
(the parens are not idiomatic in our perl)
Expanding ticket mentions can create a lot of spam. Introduce a user pref for the max number of ticket mentions to react to, so it can be limited.
838d9ac to
b3bbe99
Compare
This creates a lot of spam, so let's not.
It would maybe be more elegant to make a "would you like to expand these" question box. I'll make that a Fryday task.