Re: [PATCH] drm/scheduler: fix documentation by replacing rq_list with sched_list

From: Luben Tuikov <luben.tuikov@amd.com>
To: Nirmoy <nirmodas@amd.com>, Nirmoy Das <nirmoy.aiemd@gmail.com>,
	amd-gfx@lists.freedesktop.org
Cc: alexander.deucher@amd.com, nirmoy.das@amd.com, christian.koenig@amd.com
Subject: Re: [PATCH] drm/scheduler: fix documentation by replacing rq_list with sched_list
Date: Mon, 20 Jan 2020 10:39:54 -0500	[thread overview]
Message-ID: <97e0f042-e6e6-0d62-85d2-dec4f2434c5d@amd.com> (raw)
In-Reply-To: <8080c3c5-0d8d-5f85-011a-1edf4074eda9@amd.com>

On 2020-01-17 5:59 a.m., Nirmoy wrote:
> Hi Luben,
> 
> On 1/16/20 6:13 PM, Luben Tuikov wrote:
>>> - * Note: the rq_list should have atleast one element to schedule
>>> + * Note: the sched_list should have atleast one element to schedule
>> "atleast" --> "at least".
> Always a tricky one to catch, Thanks!
> I will create a patch and ask Alex to squash with this one.
>>
>>> - * @num_rq_list: number of run queues in the rq_list
>>> + * @sched_list: a list of drm_gpu_schedulers on which jobs from this entity can
>>> + *              be scheduled
>> I had to read this a few times to understand it. I wonder if splitting
>> it into two sentences would make it clearer:
>>
>> "A list of schedulers (drm_gpu_schedulers). Jobs from this entity,
>>   can be scheduled on any scheduler on this list."
> 
> I don't know, both feels right to me. Please create a patch if you think 
> this splitting makes it more clearer.

Oh, god, since you're submitting a patch to fix "atleast" to "at least",
it would've been good to also fix this.

But you chose not to do it. Sure, you understand it, but making it more
clear, surely shows attention to detail and thinking process. And it
would help others to maintain the driver and contribute. Making
it more obfuscated, by bunching up what acts and what, makes it difficult
to understand.

I had to read this sentence a few times to separate the entities,
what acts on what, in order to understand the description.

Bunching this up into a single sentence, invariably makes
the documentation *more difficult* to understand. Breaking it up
into two sentences, makes it much easier to understand. For instance,

The first sentences describes a single object: the list:
"A list of schedulers (drm_gpu_schedulers)."
The second sentence describes the jobs, that they can be
scheduled on any scheduler of the previously talked about list:
"Jobs from this entity, can be scheduled on any scheduler on this list."

Why would you refuse to also update the documentation, as part of your
spelling mistake patch, is beyond me. It only makes sense to make
things better for other people to understand.

And this is how well-written documentation dies.

Regards,
Luben
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx