Waiting for feedback label is removed too early in some edge cases

[dreis2211]

Hi,

I just noticed after commenting on spring-projects/spring-boot#19759 that the "waiting-for-feedback" label was removed.

In that case I didn't provide any feedback, but rather had an additional question for the issue creator in order to hopefully get closer to the root cause. I could imagine similar scenarios where other people comment on an issue with "+1" or "I have the same problem", while the original issue creator didn't really provide any feedback.

Looking through the code of FeedbackIssueListener this seems to be intended. I'm wondering if it makes sense to only check for comments of the initial issue creator. Yet, this would maybe ignore helpful feedback from others (e.g. when they have the same problem and can provide a sample).

I just wanted to report this and make you aware (in case you weren't). Maybe you have some more thoughts on this. Probably any solution (I can think of) has its edge cases, so feel free to simply close this.

Cheers,
Christoph

[wilkinsona]

Yeah, it is intentional. That said, it's only what we thought might work and wanted to try to begin with. We haven't really revisited it since so thanks for the prompt to do so.

I've certainly experienced the problem that you just experienced on the Boot issue several times. I'm not sure how often someone else has provided feedback and the bot's current behaviour has been useful. As it is useful, I feel I'm less likely to notice than when it gets it wrong. That said, my hunch is that it's less often than when it thinks feedback has been provided and it actually hasn't it.

In short, I think it's worth making a change to require feedback from a specific person and seeing how it goes. However, I'm not sure how it should work. Requiring feedback from the initial creator might not be the right person and we may need a way of indicating who we want to get feedback from. Let's worry about that once the other folks have had a chance to say if they think changing the behaviour is a good idea. /cc @rstoyanchev @philwebb @rwinch @eleftherias @vpavic @mbhave.

[rstoyanchev]

It would be a reasonable improvement, but I also think the more common case is the original reporter adding comments or questions, and it gets both noisy and laborious. Here is one recent one spring-projects/spring-framework#24353.

I am wondering whether it wouldn't make more sense to let a human decide when feedback was provided. The bot can still add "feedback-provided". The developer would then either manually drop "waiting-for-feedback" if feedback was provided, or drop "feedback-provided" if not yet in which case the bot would start to monitor again.

[wilkinsona]

Thanks, @rstoyanchev.

The developer would then either manually drop "waiting-for-feedback" if feedback was provided, or drop "feedback-provided" if not yet in which case the bot would start to monitor again.

Would that be much of an improvement? You'd still have to update the labels either to note that feedback has indeed been provided or that some is still required.

[rstoyanchev]

If the bot guesses incorrectly, it reduces some noise, e.g. the following sequence:

spring-issuemaster added "status: feedback-provided" and removed "status: waiting-for-feedback"

assignee commented ...

assignee added "status: waiting-for-feedback" and removed "status: feedback-provided"

reporter commented ...

spring-issuemaster added "status: feedback-provided" and removed "status: waiting-for-feedback"

...

would become:

spring-issuemaster added "status: feedback-provided"

assignee commented ...

assignee removed "status: feedback-provided"

reporter commented ...

spring-issuemaster added "status: feedback-provided"

...

If the bot guess correctly then assignee still has to manually update the ticket (change labels, set target milestone) and at that point dropping one extra label is negligible. You could say this ensures "waiting-for-feedback is added at most once, and also removed at most once.

Beyond that, I see it as a more accurate reflection of what's going on since the bot cannot properly determine if feedback was provided, arguably removing waiting-for-feedback is always going be imperfect and doesn't bring any value that I can see.

[wilkinsona]

That's a compelling argument, Rossen. Thanks. If I don't here any objections, I'll make that change soon and we can try it out.

[mbhave]

The only issue I see with not removing the waiting-for-feedback label when feedback-provided is added is if the team doesn't get to manually removing the label within 7 days. The bot will then add the feedback-reminder label along with a comment asking for feedback which might confuse the reporter in the case where the bot had guessed right.

[rstoyanchev]

I think when feedback-is-provided is present, the ticket is in a state where the next step must be taken by a developer and that means the clock should be off. Upon reviewing the new information, the developer would either accept/schedule, or comment and drop feedback-is-provided, which would put the clock back on.

[mbhave]

I think that would be a change we have to make as part of this issue then? IIRC the clock is based only on the presence of the waiting-for-feedback label at the moment.

[wilkinsona]

Yeah, that's right. It'd have to get a bit more sophisticated as we'd need to reset and start the clock when feedback-provided is removed and waiting-for-feedback remains. If we didn't reset it, the reporter might not get a reasonable amount of time to respond.

[mbhave]

It's tricky. The fact that we removed feedback-provided and left waiting-for-feedback on could mean that the bot guessed wrong and the original reporter didn’t actually provide feedback. It could, however, also mean that the feedback that was provided moved the discussion on but led to a different question being asked where some more feedback was needed. Ideally we'd only want to reset it in the latter case.

[rstoyanchev]

If more feedback is needed and without it the issue cannot be scheduled then the clock should be on, no? If more feedback is needed but the issue clearly can be scheduled then I don't see the need to leave waiting-for-feedback and have a clock running. So I'm not following what the concern is.

[mbhave]

If more feedback is needed and without it the issue cannot be scheduled then the clock should be on, no?

Yes. The issue is when the clock should be reset. If the original feedback-provided was accurate and more feedback is needed, the clock should be reset. If the original feedback-provided was not actual feedback from the reporter (the bot guessed wrong), then ideally the clock should not be reset.

[rstoyanchev]

Right, I got it now! Yes it would be nice to control if the clock restarts or continues, and I can imagine some additional label like comment-provided for when the clock should continue, but as you pointed out, the time taken for the team to respond should be subtracted from the overall clock time.

Beyond that even if we didn't have that sophistication I would still be okay with a simple resetting of the clock. Yes that might prolong things but as long as there is a clock that'll close eventually on its own, and it's no different from what we have today.