public inbox for [email protected]
 help / color / mirror / Atom feed
From: Eli Schwartz <[email protected]>
To: Pavel Begunkov <[email protected]>,
	Ammar Faizi <[email protected]>
Cc: Jens Axboe <[email protected]>,
	io-uring Mailing List <[email protected]>
Subject: Re: [PATCH for-next 3/3] test range file alloc
Date: Thu, 30 Jun 2022 15:26:56 -0400	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On 6/30/22 11:18 AM, Pavel Begunkov wrote:
> On 6/30/22 15:31, Ammar Faizi wrote:
>> On 6/30/22 9:19 PM, Pavel Begunkov wrote:
>>> Nobody cared enough to "fix" all tests to use those new codes, most
>>> of the cases just return what they've got, but whatever. Same with
>>> stdout vs stderr.
>>
>> That error code rule was invented since commit:
>>
>>     68103b731c34a9f83c181cb33eb424f46f3dcb94 ("Merge branch
>> 'exitcode-protocol' of ....")
>>
>>     Ref: https://github.com/axboe/liburing/pull/621/files
>>
>> Thanks to Eli who did it. Eli also fixed all tests. Maybe some are still
>> missing, but if we find it, better to fix it.
> 
> Have no idea what you're talking about but I'm having
> hard time calling 6 returns out of 21 in this file "all".


Hi, I should probably clarify the state of affairs...

I submitted a patch series on github 4 days ago which implements those
new codes. It was merged 2 days ago. This is very new code, so I think
it's not completely 100% fair to say that no one "cared" enough to use it.

As far as the actual changes and their completion go... take a look at
the commit messages in the merged patches, specifically take a look at
commit ed430fbeb33367324a039d9cee0fd504bb91e11a.

"""
tests: migrate some tests to use enum-based exit codes

[...]

A partial migration of existing pass/fail values in test sources is
included.
"""

You can also take a look at Github's equivalent of a cover letter, in
which I mentioned that I haven't ported everything, but what I did do is
still useful because "a) it has to start somewhere, b) it demonstrates
the basic idea of how to structure things."

As far as I'm concerned, I believe the patch series stands on its own
merit. I established the framework to use, and that on its own is useful
and deserves merging, because it means that people can start using it,
and getting things correct from the beginning when adding new code.

Old code does need to be carefully checked, it's not a simple
find/replace, but that can be done incrementally, and I'm willing to
continue work on that myself. I just don't think it has to be all or
nothing at the time of merging.


...

Also, for the record -- while waiting for the Github patch series to be
merged, I did continue to convert more code via git commit --fixup= &&
git rebase -i --autosquash. If it had taken longer to end up being
merged, I would have ended up converting more tests over, and that would
have reflected on the current state of git master.

I'm not sad that it got merged when it was, because again, this work can
be done incrementally and people can take advantage of existing work
immediately. Jens decided it was ready to merge, and that seems like a
fine decision to me. If he had asked me to finish porting all the tests
first, I could have done that too.

More will get done in short order. I'm not even a bottleneck for doing
it. :)

(Though I will work on it.)


-- 
Eli Schwartz


  reply	other threads:[~2022-06-30 19:27 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-30  9:13 [PATCH for-next 0/3] ranged file slot alloc Pavel Begunkov
2022-06-30  9:13 ` [PATCH for-next 1/3] update io_uring.h with file slot alloc ranges Pavel Begunkov
2022-06-30  9:13 ` [PATCH for-next 2/3] alloc range helpers Pavel Begunkov
2022-06-30 13:08   ` Jens Axboe
2022-06-30  9:13 ` [PATCH for-next 3/3] test range file alloc Pavel Begunkov
2022-06-30 13:09   ` Jens Axboe
2022-06-30 13:32     ` Ammar Faizi
2022-06-30 14:19     ` Pavel Begunkov
2022-06-30 14:31       ` Ammar Faizi
2022-06-30 15:18         ` Pavel Begunkov
2022-06-30 19:26           ` Eli Schwartz [this message]
2022-06-30 20:03             ` Jens Axboe
2022-06-30 14:57       ` Jens Axboe
2022-06-30  9:18 ` [PATCH for-next 0/3] ranged file slot alloc Pavel Begunkov

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 \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /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