=?utf-8?q?Donát?= Nagy <donat.n...@ericsson.com>, =?utf-8?q?Donát?= Nagy <donat.n...@ericsson.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/70...@github.com>
DonatNagyE wrote: Thanks for the suggestions; I'll probably upload the next revision tomorrow. > One remark about the review workflow, I'd prefer if the conversation starter > would resolve the conversations. Let me explain why: > > Given the amount of reviewers for CSA, I'd suggest explicitly supporting > reviewer experience. Speaking of that, personally I find the following > workflow to suite myself the best: > > * All comments needs a reaction, either by explict commenting on it or by > putting there a thums-up emoji or something. This helps the author follow > which comments were addressed downstream and is pending for submission. An > explicit comment is expected for challenging a comment. I prefer emojies, > becase they don't send an email to everyone subscribe - unlike for individual > comments; and usually I also batch individual commits into a "self-review" as > patch author when I reply for the same reason. > > * All comments needs to be reacted before requestion the next round of review > from the person who added those comments. > > * The comment should be only marked resolved by the person who raised the > concern. This ensures that the comment was adequately solved, thus the issue > is no longer relevant. > > * To merge a PR, the PR should have no open conversations. Post-approval > comments are also fine and leaves room for further discussions, but the goal > is no longer to directly challenge the legitimacy of the patch. Sounds reasonable, I'll try to follow this process, especially when interacting with you. In the previous round I resolved the conversations because I applied them (mostly verbatim) in the new revision that I uploaded; but I can instead apply a thumbsup emoji if you'd prefer that. https://github.com/llvm/llvm-project/pull/70056 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
