This message was deleted.
# announcements
s
This message was deleted.
a
If you did re-request everyone who commented/requested changes, it shouldn't be in returned to you. Let me know if this continues to happen
v
There is indeed one person who commented and I didn't re-request a review, because it was not a postive or negative review, it was just a comment. Isn't it wrong to consider these in that case?
Especially, as you can also not discard a "comment" review
a
I personally agree with you, but there are a lot of companies that don't use request changes due to it being perceived as too aggressive, comments there are the method of communicating that something needs to be addressed. I think what we can do better though is make it clear how to customize this behavior for yourself. cc @Xiulung Choy / @merrill on what brings something into "Returned to you"
v
To customize it I guess I (everyone) would need to change the filters of that section?
a
Yes exactly. But we would need to make sure the filters match across all sections, because if it is filtered out of "Returned to you", it should still show up in another section
Right now we try to make the sections relatively MECVE
MECE*
v
Looking at the filters, there are three filters for that section. All three have in common the conditions "not draft, status open, author @me, not currently merging". And then they have any closed review request && not required approvals || any closed review request && has no approval || any closed review request with changes requested tbh I still don't get from these, why it is in there with the "comment only" review. That review was not requested. Only the review from leonard was requested. He closed the review request with requested changes. And his review was re-requested.
If the review of Lőrinc Pap would have been requested and he reviews with comment, I would agree (or at least accept) that it is in "returned to you" section.
But like it is I still don't really get the logic or how I would adapt the conditions
a
Ah i see, the commenter wasnt a requested reviewer
Yeah I agree with you
That shouldn't put it in returned to you
Let me see if we can fix that
❤️ 1
v
Actually, if a review was not requested, but nevertheless submitted with "changes requsted" I would also expect it to be in "returned to you" semantically. Though how I read the filters / conditions I'm not sure whether it should end up in there technically. Reviews are not always explicitly requested, but just done by appropriate persons, or requested from multiple persons and only some or one do it, or requested by some but done by others. So maybe the logic should be "changes requested || (review requested && commented)", and additionally the condition renamed as the left-hand side of the "or" does not match "any closed review request" which is part of all three filter-cases. 🙂
Hm, or maybe it is ok like it is. In the case of https://github.com/spockframework/spock/pull/1689 szpak who was not asked for a review put a comment-only review to make a suggestion and ask for what I think. There it feels correct that it is in "Returned to you" 😕
a
Yeah -- I wish GitHub was simpler wrt the possible states you can put a PR in (like Phabricator which has a global state on the PR). It's pretty hard to neatly put things into sections, but we tried to be as inclusive as we could. I think the next step here is improving the filtering experience so it is easier to customize.
v
Definitely. 🙂 Maybe also with some option to see / compare to (no reset to) the current default configuration, in case you make improvements that someone might be interested to get, maybe together with some notification that the default configuration was changed. :-)