Re: [PATCH v2 0/8] fast-import: tighten parsing of paths

From: Thalia Archibald <thalia@archibald.dev>
Date: Apr 8, 2024, 07:15 UTC
Message-ID: <1B15EC5E-E0E7-474E-A0D8-3089918394A5@archibald.dev>
In-Reply-To: <ZhONyBIFlWbNHNwN@tanuki>

On Apr 7, 2024, at 23:25, Patrick Steinhardt <ps@pks.im> wrote:

Show 15 quoted lines

> On Sun, Apr 07, 2024 at 07:46:52PM -0400, Eric Sunshine wrote:
>> 
>> Lack of response may be due to the series being overlooked, or it
>> could mean that nobody has any particular interest in the changes
>> (which is not to say that there is anything wrong with the changes),
>> or that people are simply busy elsewhere. It could also mean that this
>> reroll is good enough and reviewers have nothing else to add. So,
>> cc'ing potentially interested people makes sense.
> 
> Yeah, for this patch series I think it's mostly a lack of interest.
> Which is too bad, because it does address some real issues with
> git-fast-import(1). Part of the problem is also that this area does not
> really have an area expert at all -- if you git-shortlog(1) for example
> "builtin/fast-import.c" for the last year you will see that it didn't
> get much attention at all.

Unfortunately, my upcoming patches will probably suffer the same fate, as they're mostly fixing parsing issues in fast-import.

Show 9 quoted lines

> Anyway, I'm currently trying to make it a habit to pick up and review
> random patch series that didn't get any attention at all every once in a
> while, which is also why I reviewed your first version. I'm taking these
> a bit slower though, also in the hope that some initial discussion may
> motivate others to chime in, as well. Which may explain why I didn't yet
> review your v2.
> 
> In any case I do have it in my backlog and will get to it somewhen this
> week.

Thank you!

Show 12 quoted lines

>>> I have several more patch sets in the works, that I’ve held back on sending
>>> until this one is finished, in case I’ve been doing something wrong. I want to
>>> move forward. Thank you for your time.
>> 
>> If the additional patch sets are unrelated to this patch set, then I
>> don't see a reason to hold them back. Feel free to send them. Even if
>> they are related to this patch set, you may still want to send them.
>> After all, doing so may get the ball rolling again on this patch set.
> 
> Agreed. Especially given that this is your first contribution, the
> quality of your patch series is quite high. So I don't see much of a
> reason to hold back the other patch series in case they are unrelated.

My effort comes from reimplementing fast-import parsing as a Rust library, following the implementation, not just the documentation, so I’ve noticed many mismatches between the concrete and abstract grammars. Perhaps it would save reviewer time to submit those around the same time, so knowledge of fast-import needs to be evicted and loaded from cache less.

Thalia