[PATCH v4 4/9] models: add addressed field

Stephen Finucane stephen at that.guru
Sat Aug 21 08:27:54 AEST 2021


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 <raxel at google.com>
> Reviewed-by: Daniel Axtens <dja at 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



More information about the Patchwork mailing list