[Development] Nominating for approver: commentby vs reviewedby vs reviewer search operators

Mårten Nordheim marten.nordheim at qt.io
Tue Aug 15 16:55:13 CEST 2023


Hey!

Yeah, I think reviewer: is sometimes a bit misleading, so +1 for changing to commentby:
Though, in that case we might want to also, for transparency, add "-owner:<nominated>". It's only natural
to reply on your own changes of course :)


Mårten

________________________________________
From: Development <development-bounces at qt-project.org> on behalf of Volker Hilsheimer via Development <development at qt-project.org>
Sent: Tuesday, August 15, 2023 16:47
To: development at qt-project.org
Subject: [Development] Nominating for approver: commentby vs reviewedby vs reviewer search operators

Hi,

Emails in which we nominate contributors for Approver status usually have two queries: the list of changes owned [1], and the list of changes that the nominee reviewed [2].

[1] https://codereview.qt-project.org/q/owner:volker.hilsheimer%2540qt.io
[2] https://codereview.qt-project.org/q/reviewer:volker.hilsheimer%2540qt.io

The first query should be pretty clear: all changes I owned, which is usually all changes I authored.

However, the second query selects all changes on which I was added as a reviewer, no matter whether I actually provided any input. So the list produced by [2] includes

https://codereview.qt-project.org/c/qt/qtdeclarative/+/496546

even though I didn’t provide any input to this change.

Being added as a reviewer by other contributors is a sign of trust. However, for the sake of evaluating a nomination, perhaps we want to get an idea of how much someone actually participated in code reviews.

Gerrit provides two selectors to get a list of changes where someone actually participated: reviewedby [3] and commentby [4]:

[3] https://codereview.qt-project.org/q/reviewedby:volker.hilsheimer%2540qt.io
[4] https://codereview.qt-project.org/q/commentby:volker.hilsheimer%2540qt.io

The former lists all the changes for which I voted on any of the patch sets (it doesn’t have to be the patch set that ultimately got merged). The latter lists all changes where I made a comment. The list produced by [3] doesn’t include https://codereview.qt-project.org/c/qt/qtbase/+/489071 (even though I commented), while the list produced by [4] does.

Commenting is very valuable, even when not voting. And voting implies commenting, e.g. the list from [4] does include https://codereview.qt-project.org/c/qt/qtopcua/+/496329, where I only voted but didn’t leave a comment.

Hence, I’d like to propose that nomination emails include the two links as per [1] and [4], using `owner` and `commentby` search operators.


Volker

--
Development mailing list
Development at qt-project.org
https://lists.qt-project.org/listinfo/development


More information about the Development mailing list