DPDK CI discussions
 help / color / mirror / Atom feed
From: Aaron Conole <aconole@redhat.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: Michael Santana <msantana@redhat.com>,
	 ci@dpdk.org,  Dumitru Ceara <dceara@redhat.com>,
	 David Marchand <dmarchan@redhat.com>,
	 Ilya Maximets <imaximet@redhat.com>
Subject: Re: [RFC 2/2] recheck: Add a recheck parser for patchwork comments
Date: Thu, 02 Nov 2023 09:03:35 -0400	[thread overview]
Message-ID: <f7tedh871eg.fsf@redhat.com> (raw)
In-Reply-To: <1912626.taCxCBeP46@thomas> (Thomas Monjalon's message of "Thu, 02 Nov 2023 11:44:53 +0100")

Thomas Monjalon <thomas@monjalon.net> writes:

> 01/11/2023 20:16, Aaron Conole:
>> Michael Santana <msantana@redhat.com> writes:
>> > I like this workflow. The only thing that I do not like is that you
>> > have to check every comment on every patch. That seems like an
>> > expensive operation, but honestly I do not think there is a better way
>> > to accomplish this. So if there is no better way to do it then it's
>> > okay, let's move forward with it
>> 
>> There isn't a different way to do it for now, but I hope to switch to
>> using the events API which should mean we only look at the most recent
>> events that come in.
>
> What prevent us to use the events API?

Ideally we could use it everywhere, but the pw-ci project is used for
other patchwork instances.  Events API for comments is a recent change,
and not every patchwork instance is upgraded to support it (for example,
both ozlabs and kernel.org patchwork instances don't have support).

I do have some detection code, and am planning on hooking that up so
that we can detect whether events API supports comment events based on
the filters offered, but that takes some time to test and validate.

So it becomes a question of which is more important - having something
working now, or spending time with the detection code.  Either way, we
need it for older patchworks that haven't upgraded to the just released
version (some projects are still on 2.2.0).

If you think it is better to do the events path first, I can go with
that but then we severely limit which projects get support for rechecks,
and I've already gotten the feature request for both OVS and OVN - so
we'd either need to support comments polling anyway, or do the massive
work of upgrading ozlabs instance.


  reply	other threads:[~2023-11-02 13:03 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-27 13:06 [RFC 1/2] pw_mon: improve command line options Aaron Conole
2023-10-27 13:06 ` [RFC 2/2] recheck: Add a recheck parser for patchwork comments Aaron Conole
2023-11-01 16:57   ` Michael Santana
2023-11-01 19:16     ` Aaron Conole
2023-11-02 10:44       ` Thomas Monjalon
2023-11-02 13:03         ` Aaron Conole [this message]
2023-11-02 13:32           ` Thomas Monjalon
2023-10-31 14:45 ` [RFC 1/2] pw_mon: improve command line options Michael Santana
2023-10-31 15:54   ` Aaron Conole

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f7tedh871eg.fsf@redhat.com \
    --to=aconole@redhat.com \
    --cc=ci@dpdk.org \
    --cc=dceara@redhat.com \
    --cc=dmarchan@redhat.com \
    --cc=imaximet@redhat.com \
    --cc=msantana@redhat.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).