Hi Andrej,
On 3/26/20 10:54 AM, Andrej Butok via TF-A wrote:
But I am worried that a self-review is rarely as good as a peer review
On practice, unfortunately, some TF-M tasks are waiting weeks and even months for review and following approvals. If I were a maintainer & owner of my own TFM area, I do not want to wait & push & remind somebody else. Better to have a post-merge review for these cases, which does not limit and slow down the development.
Thanks for the feedback. That's not good, patches can't realistically stay in review for weeks and even months, that's just not workable. Worse, it might discourage developers to contribute to the project.
I can see that cumulating maintainer & owner roles would solve the problem here but perhaps enlarging the pool of maintainers would as well? Presumably, the situation is like that today because the current maintainers of the project are overloaded and cannot get all reviews done in a timely manner?
I am skeptical about a post-merge review process... Once a patch is merged there is less urge and motivation (if any) for people to take a look at it. I am worried that patches might never get reviewed that way.
Regards, Sandrine
Hi Sandrine,
The motivation is the same for the both cases - no motivation ;)
That is why I prefer for my own supported area, if it does not touch a foreign code, to be able to do a self-review and to merge it to the main-line. In addition, I may request somebody to do the post-commit review, but it does not create any obstacles to continue the development, it can be useful for the following improvement discussion/suggestions or proposals.
OR as a compromise, should be added a possibility to set a review deadline after which it's automatically marked as approved.
Thanks, Andrej Butok
-----Original Message----- From: Sandrine Bailleux sandrine.bailleux@arm.com Sent: Wednesday, April 1, 2020 10:33 AM To: Andrej Butok andrey.butok@nxp.com; tf-a tf-a@lists.trustedfirmware.org; tsc@lists.trustedfirmware.org; op-tee@linaro.org Cc: nd@arm.com Subject: Re: [TF-A] [TF-M] Project Maintenance Proposal for tf.org Projects
Hi Andrej,
On 3/26/20 10:54 AM, Andrej Butok via TF-A wrote:
But I am worried that a self-review is rarely as good as a peer review
On practice, unfortunately, some TF-M tasks are waiting weeks and even months for review and following approvals. If I were a maintainer & owner of my own TFM area, I do not want to wait & push & remind somebody else. Better to have a post-merge review for these cases, which does not limit and slow down the development.
Thanks for the feedback. That's not good, patches can't realistically stay in review for weeks and even months, that's just not workable. Worse, it might discourage developers to contribute to the project.
I can see that cumulating maintainer & owner roles would solve the problem here but perhaps enlarging the pool of maintainers would as well? Presumably, the situation is like that today because the current maintainers of the project are overloaded and cannot get all reviews done in a timely manner?
I am skeptical about a post-merge review process... Once a patch is merged there is less urge and motivation (if any) for people to take a look at it. I am worried that patches might never get reviewed that way.
Regards, Sandrine
Hi Andrej,
On 4/1/20 10:55 AM, Andrej Butok wrote:
Hi Sandrine,
The motivation is the same for the both cases - no motivation ;)
My hope with this proposal would be to turn the motivation problem into a responsibility problem. Perhaps today the expectations are too implicit and this leads to people having to volunteer, which is based on good willing and self-motivation?
If we set clear expectations about the responsibilities of code owners and maintainers, if we set clear timelines for people to review and approve patches in a certain amount of time, then I am hoping we will see less situations where patches don't get enough attention. I think we have to make developers responsible and if they are not fulfilling the expectations anymore (because they no longer have enough motivation or bandwidth or whatever) then their role within the project (code owner/maintainer) ought to be revised.
Does that sound reasonable?
That is why I prefer for my own supported area, if it does not touch a foreign code, to be able to do a self-review and to merge it to the main-line. In addition, I may request somebody to do the post-commit review, but it does not create any obstacles to continue the development, it can be useful for the following improvement discussion/suggestions or proposals.
OR as a compromise, should be added a possibility to set a review deadline after which it's automatically marked as approved.
I like your last suggestion. And that echoes to others' suggestions (Christian Daudt, Joakim Bech).
Regards, Sandrine
-
Andrej Butok -
Sandrine Bailleux