Re: [PATCH] rust: macros: improve `#[vtable]` documentation

[Benno Lossin]

On 12.10.23 15:48, Wedson Almeida Filho wrote:
> On Thu, 12 Oct 2023 at 10:22, Benno Lossin <benno.lossin@xxxxxxxxx> wrote:
>> +
>> +/// Error message for calling a default function of a [`#[vtable]`](macros::vtable) trait.
>> +pub const VTABLE_DEFAULT_ERROR: &str =
>> +    "This function must not be called, see the #[vtable] documentation.";
>> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
>> index c42105c2ff96..dab9a1080b82 100644
>> --- a/rust/macros/lib.rs
>> +++ b/rust/macros/lib.rs
>> @@ -87,27 +87,41 @@ pub fn module(ts: TokenStream) -> TokenStream {
>>   /// implementation could just return `Error::EINVAL`); Linux typically use C
>>   /// `NULL` pointers to represent these functions.
>>   ///
>> -/// This attribute is intended to close the gap. Traits can be declared and
>> -/// implemented with the `#[vtable]` attribute, and a `HAS_*` associated constant
>> -/// will be generated for each method in the trait, indicating if the implementor
>> -/// has overridden a method.
>> +/// This attribute closes that gap. A trait can be annotated with the `#[vtable]` attribute.
>> +/// Implementers of the trait will then also have to annotate the trait with `#[vtable]`. This
>> +/// attribute generates a `HAS_*` associated constant bool for each method in the trait that is set
>> +/// to true if the implementer has overridden the associated method.
>> +///
>> +/// If you want to make a function optional, you must provide a default implementation. But this
> 
> We should  standardise how we write our documentation. In the `rust`
> branch, we avoided using the imperative mood like you have here; the
> rationale was that the documentation was describing how things
> are/work, so we shouldn't be giving orders to readers (they may be
> authors of traits, implementers of some vtable trait, or neither, just
> someone learning about things, etc.).
> 
> In the paragraph above, you'll find an example: "Implementers of the
> trait will then also have to...".
> 
> For the specific case above, I'd suggest: 'For a function to be
> optional, it must have a default implementation.', or using the
> passive voice, 'To make a function optional, a default implementation
> must be provided'.

I agree, I also think we should just fix this now, so I will send a v2.

-- 
Cheers,
Benno