[PATCH] Add comments REST API

[Veronika Kabatova]

----- Original Message -----
> From: "Daniel Axtens" <dja at axtens.net>
> To: "Stephen Finucane" <stephen at that.guru>, "Veronika Kabatova" <vkabatov at redhat.com>
> Cc: patchwork at lists.ozlabs.org
> Sent: Tuesday, April 10, 2018 4:56:59 PM
> Subject: Re: [PATCH] Add comments REST API
> 
> Stephen Finucane <stephen at that.guru> writes:
> 
> > On Mon, 2018-04-09 at 09:29 -0400, Veronika Kabatova wrote:
> >> ----- Original Message -----
> >> > From: "Stephen Finucane" <stephen at that.guru>
> >> > To: vkabatov at redhat.com, patchwork at lists.ozlabs.org
> >> > Sent: Monday, April 9, 2018 3:02:57 PM
> >> > Subject: Re: [PATCH] Add comments REST API
> >> > 
> >> > On Fri, 2018-03-23 at 13:33 +0100, vkabatov at redhat.com wrote:
> >> > > From: Veronika Kabatova <vkabatov at redhat.com>
> >> > > 
> >> > > Signed-off-by: Veronika Kabatova <vkabatov at redhat.com>
> >> 
> >> Thanks for feedback!
> >> 
> >> > Looks pretty good. As you note, it does need some tests. I'm also
> >> > curious about the idea of nesting this under the patches endpoint. Let
> >> > me know what you think.
> >> > 
> >> 
> >> Hmm, that would require the same change for cover letters too. How do
> >> you imagine it? Having all related comments under the detailed patch or
> >> cover, or something else?
> >
> > Indeed. I'd expect all replies to a given patch to be under that patch
> > and so on. This is what we present in the web UI so it probably makes
> > sense to do the same for the REST API. Unless there's a strong reason
> > _not_ to, this is the way I'd go, yeah.
> >
> >> I opted for separate API because some people may not care about comments
> >> (since AFAIK noone requested it yet), but we very much do. At the same
> >> time I didn't want to push our needs on others. If you don't think it's
> >> an issue I can rework this and put it under the patch/cover endpoints.
> >
> 
> We do have 1 pending request re: the comments API:
> https://github.com/getpatchwork/patchwork/issues/143
> 
> I don't know how that would interact with either design, but I wanted to
> flag it as a user story to consider.
> 

Thanks for posting that! I checked the issue list briefly but the subject
didn't seem relevant so I missed it. Taking this feature into consideration:

(previous solution) keeping the API as /comments, the tools can filter
  through it and then send requests for patch updates
(currently working on) putting the comments into patches / covers
  endpoints as a serialized list, tools can query the patch and check the
  comments there directly, then send the request to the same /patch/ID
  endpoint used previously

It would be the best if those changes were triggered automatically with
comment parsing but until we have a safe way of doing that and a proof of
concept, having a tool run periodically and marking patches should work
fine with both implementations.

I don't think the hidden endpoint Stephen mentioned below will play well
with this request or add anything useful - we have a hard time getting
checks associated with given patch since the link from patch goes to the
list of all checks which can't be filtered by patch ID (instead of listing
checks associated with the patch directly). Whichever way we go with the
comments API implementation, the hidden endpoint will be more confusing
than useful, IMHO.


I'll go ahead and implement the changes discussed previously if you guys
don't have additional ideas.

Thanks,
Veronika


> Regards,
> Daniel
> 
> > Yup, I didn't do it because I wasn't sure how helpful it was. I'm a-ok
> > adding it though so go for it.
> >
> > Stephen
> >
> > PS: You should probably put a 'comments' url in the 'patches' and
> > 'covers' resources, similar to what we do for 'checks' of 'patches'.
> > _______________________________________________
> > Patchwork mailing list
> > Patchwork at lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/patchwork
>