Hi Bobby, 1) What is the advantage of abandoning changes?
Could reviewers just remove themselves from the reviewer list if they're not interested in the patch? Maybe mark as WIP instead of abandoning. I generally dislike when websites / communities close my stuff. ________________________________ From: gem5-dev <gem5-dev-boun...@gem5.org> on behalf of Bobby Bruce <bbr...@ucdavis.edu> Sent: Friday, October 4, 2019 7:21 PM To: gem5 Developer List <gem5-dev@gem5.org> Subject: [gem5-dev] Updating Contribution.md review guidelines. Feedback appreciated. Dear all, I'm currently in the process of updating CONTRIBUTING.md with some Gerrit Code Review guidelines (please see my WIP change here: https://gem5-review.googlesource.com/c/public/gem5/+/21419 ). My reason for doing this is finding out no one really knows the procedure for addressing some common problems in Gerrit. I think having some written-down guidelines, understood by both reviewers and contributors, will help us overcome this. I outline here the three problems I've observed in how we currently work with Gerrit and how these guidelines help: 1) It doesn't seem like we have a good policy for handling old, active changes. It's all too common for a change to be uploaded, a few rounds of patchsets to be submitted, then the original contributor disappears. It seems like, at present, no one knows how to handle this in a consistent way. To address this I've added a guideline that after a month of inactivity from the original contributor, reviewers can reach out and ping contributors on a change. If after 48 hours of no response (or confirmation the contributor is no longer interested), reviewers can take ownership or abandon the change. Whether to abandon or take ownership will need to be decided on a case-by-case basis. I hope with this policy we can reduce the number of open changes in Gerrit over time. 2) I've observed that sometimes a change, for whatever reason, enters a "Ready to Submit" state and stays there for a while. I found a couple of instances of old "Ready to Submit" changes (i.e., >1 month). It's odd and as time goes on, the chances of this "Ready to Submit" change incurring a merge conflict increases. I've therefore added a guideline that the original contributor of a change has 48 hours to give a final sign-off and submit once the change reaches a "Ready to Submit" state. After this period a reviewer may do so on the original contributors behalf. 3) Some have noted to me that they are unsure what the procedure is for submitting a patchset to a change in which they are not the owner (commonly occurring in the "It'll take more time for me to explain what to do than just do it myself" scenario). I think it's best-practice to have the original contributor own the change but others may contribute patchsets after asking permission or for minor changes such as typos or formatting issues (i.e., completely inoffensive changes). There is some embedded flexibility here but, by and large, i think communication and consent is key. Outside of this, the remaining guidelines are basically common-sense reminders (i.e., be polite, please address every piece of feedback a given, etc.), which should help remind new contributors on how to manage the process. Please feel free to let me know what you think of these changes and if they could be improved upon (I'm also open to adding more guidelines if anyone has noticed some room for improvement). Kind regards, Bobby _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
