On Fri, 2021-08-20 at 04:50 +0000, Raxel Gutierrez wrote: > Currently, there is no state or status associated with comments. In > particular, knowing whether a comment on a patch or cover letter is > addressed or not is useful for transparency and accountability in the > patch review and contribution process. This patch is backend setup for > tracking the state of patch and cover comments. > > Add `addressed` boolean field to patch and cover comments to be able to > distinguish between unaddressed and addressed comments in the > patch-detail page. > > Signed-off-by: Raxel Gutierrez <ra...@google.com> > Reviewed-by: Daniel Axtens <d...@axtens.net>
I'm still not entirely happy with this design. It's functional, but it seems overly verbose for what I think we're trying to do here. In particular, the idea that every single comment is actionable and is unaddressed by default feels wrong to me. Who is this field intended for? Is it for maintainers to track whether a question they asked has been answered, or is it for submitter to track whether they've answered all of a reviewers questions? I suspect it's the former and, if so, what are your thoughts on defaulting to addressed being unset by default (i.e. NULL or no action item). This would mean maintainers would be required to set 'addressed=False' when needed? We could event allow them to do this via an email header which would be easily set in mail clients like mutt. If it's the latter, the same approach would probably still work, I think. WDYT? Aside from this design question, the rest of the changes, particularly those to the REST API look okay, but I'll need to spend more time on this over the weekend. Cheers, Stephen _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork