From: Siddhesh Poyarekar <siddhesh@gotplt.org>
To: Carlos O'Donell <carlos@redhat.com>
Cc: GNU C Library <libc-alpha@sourceware.org>
Subject: Re: New patchwork state: Invalid
Date: Tue, 20 Sep 2022 08:08:23 -0400 [thread overview]
Message-ID: <f0b5c322-c2d3-3693-9c2a-f718051a729d@gotplt.org> (raw)
In-Reply-To: <YymXFIEIsMRGT8tq@fedora>
On 2022-09-20 06:33, Carlos O'Donell wrote:
> On Mon, Sep 19, 2022 at 05:49:30PM -0400, Siddhesh Poyarekar wrote:
>> Adhemerval, Florian and I noticed a couple of manpage patches during review
>> that managed to apply to the glibc tree because they only added new files
>> and as a result, didn't fail CI. I've added a new terminal state 'Invalid'
>> to patchwork to mark such patches from now on. It should be used for any
>> patches that don't belong to glibc, e.g. because they've been cross-posted.
>
> I noticed this too! I asked DJ aobut his and he said it was common.
>
> I'm OK with "Invalid" but I would rather go with "Not applicable"
> because the patch is *valid* but not applicable. Total bikeshed.
>
>> This is now separate from the 'Fails to apply' state, which was previously
>> overloaded to indicate non-glibc patches.
>
> Our maxim should be: As few states as possible.
>
> My opinion is that we should not have "Fails to apply."
>
> Instead the state should always be "Failed CI."
>
> Applying a patch is a lower level operation that is related to
> pre-commit CI.
>
> The flow could be like this:
> - A patchwork bot attempts to re-apply the patch, and if it fails it
> creates a "fail" entry in the CI which reads "Rebase bot failed".
> - A patchwork bot notices the fail and marks the patch or whole
> series as "Failed CI"
>
> My suggestion is as follows:
>
> - Remove "Fails to apply." from the workflow. As few states as possible
> should be our goal.
> - Suggest renaming "Invalid" to "Not applicable" only because it sounds
> kinder (complete bikeshed).
lol, so we had a "Not applicable" before, which indicated what I added
as "Invalid". This is because you and I changed "Not applicable" to
"Fails to apply" a couple of weeks ago because I had misremembered "Not
applicable" as being "Does not apply" ;)
I'm fine with reinstating what we had before (i.e. "Not applicable") and
merge "Fails to apply" with "Fails CI". I'll make that change in the
wiki and patchwork today.
Sid
prev parent reply other threads:[~2022-09-20 12:08 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-19 21:49 Siddhesh Poyarekar
2022-09-20 10:33 ` Carlos O'Donell
2022-09-20 12:08 ` Siddhesh Poyarekar [this message]
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=f0b5c322-c2d3-3693-9c2a-f718051a729d@gotplt.org \
--to=siddhesh@gotplt.org \
--cc=carlos@redhat.com \
--cc=libc-alpha@sourceware.org \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).