----- Original Message ----- > From: "Daniel Axtens" <d...@axtens.net> > To: "Stephen Finucane" <step...@that.guru>, "Veronika Kabatova" > <vkaba...@redhat.com> > Cc: patchwork@lists.ozlabs.org > Sent: Tuesday, April 10, 2018 4:56:59 PM > Subject: Re: [PATCH] Add comments REST API > > Stephen Finucane <step...@that.guru> writes: > > > On Mon, 2018-04-09 at 09:29 -0400, Veronika Kabatova wrote: > >> ----- Original Message ----- > >> > From: "Stephen Finucane" <step...@that.guru> > >> > To: vkaba...@redhat.com, patchwork@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, vkaba...@redhat.com wrote: > >> > > From: Veronika Kabatova <vkaba...@redhat.com> > >> > > > >> > > Signed-off-by: Veronika Kabatova <vkaba...@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@lists.ozlabs.org > > https://lists.ozlabs.org/listinfo/patchwork > _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork