On Tue, Aug 31, 2021 at 05:42:20AM -0500, Alex Elder wrote:
On 8/31/21 3:07 AM, Johan Hovold wrote:
On Mon, Aug 30, 2021 at 08:20:25AM -0500, Alex Elder wrote:
I have been offering review feedback on this patch for three reasons:
- First, because I think the intended change does no real harm to the Greybus code, and in a small way actually simplifies it.
You leave out that we've already seen three versions of the patch that broke things in various ways and that there was still work to be done with respect to the commit message and verifying the locking. That's all real costs that someone needs to bear.
This is true. But it's separate from my reason for doing it, and unrelated to the suggested change.
I was perhaps reading the "no harm" bit too literally, but I'd say it very much applies to the suggested change (which was the example I used).
- Because I wanted to encourage Fabio's efforts to be part of the Linux contributor community.
Helping new contributers that for example have run into a bug or need some assistance adding a new feature that they themselves have use for is one thing.
I'm not so sure we're helping either newcomers or Linux long term by inventing work for an already strained community however.
[ This is more of a general comment and of course in no way a critique against Fabio or a claim that the XArray conversions are pointless. ]
Yes, yours is a general comment. But I would characterize this as Fabio "scratching an itch" rather than "inventing new work."
Just to clarify again, my comment was in no way directed at Fabio or not necessarily even at the XArray conversions if it indeed means that IDR/IDA can be removed.
The strained community needs more helpers, and they don't suddenly appear fully-formed; they need to be cultivated. There's a balance to strike between "I see you need a little guidance here" and "go away and come back when you know how to do it right."
And here's where I think the invented work bit really comes in. I have no problem helping someone fix a real problem or add a feature they need, but spending hours on reviewing changes that in the end no one needs I find a bit frustrating. My guess is that the former is also more likely to generate long-term contributors than teaching people C on made-up tasks or asking them to silence checkpatch.pl indentation warnings.
In any case, I don't disagree with your general point, but we seem to view this particular instance differently.
Perhaps I shouldn't have brought up the general issue in this case. If there was a general consensus that IDR was going away and some precedence outside of staging that could be used as a model, then this change would be fine.
Johan