Re: [PATCH v2 4/8] drm/xe/kunit: Add xe_kunit_helper_is_live_test()

[Gustavo Sousa <gustavo.sousa@intel.com>]

Jani Nikula <jani.nikula@linux.intel.com> writes:

> On Fri, 08 May 2026, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>> +/**
>> + * xe_kunit_helper_is_live_test - Return true if @test is a live test.
>> + * @test: the &kunit test
>> + *
>> + * Return: True for a live test and false otherwise.
>> + */
>
> Pardon me for being blunt, but I think this is the worst kind of
> kernel-doc comment.

I appreciate the bluntness! :-)

>
> It doesn't provide any additional information to what the function name
> and signature already convey (which is to say excellent job on naming
> the function), but it fails to explain what "live test" means.

I kind of just added this kernel-doc to fill a hole for "consistency",
but, yeah, it does not provide any new info.

>
> The extra bits of useful information people might need after seeing the
> function xe_kunit_helper_is_live_test() in code are: What is a live
> test, and what is it if it's not live? Dead?

Zombie? ;-)

Joking apart, I personally tend to use "regular" to refer to non-live
tests. I do agree we are missing some documentation on the subject.  I'm
not sure though this function should be the place to do it.  I think we
would be better off with a "DOC:" section for that (and also explain
other bits in there).  I think it would be sensible to rename
xe_kunit_helpers.c to simply xe_kunit.c and add such a section.

With that in place, this function would be kind of self-explanatory,
right? Is this a case we just drop the kernel-doc?

Or should we try to be consistent on "every public function should have
a kernel-doc"?  Is that even a rule or am I imagining things? :-)

--
Gustavo Sousa

>
>
> BR,
> Jani.
>
>> +bool xe_kunit_helper_is_live_test(struct kunit *test)
>> +{
>> +	KUNIT_STATIC_STUB_REDIRECT(xe_kunit_helper_is_live_test, test);
>> +	return false;
>> +}
>
> -- 
> Jani Nikula, Intel