Re: [PATCH RFC 07/18] rust: drm: mm: Add DRM MM Range Allocator abstraction

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Apr 6, 2023 at 5:45 PM Daniel Vetter <daniel@xxxxxxxx> wrote:
>
> Yeah this all looks great and very hyperlinked.
>
> I think the only nit I have is that for types with two or more type
> variables (like the rbtree) what each of them should represent in the top
> intro. I can guess it's <Key, Value> and not the other way round, but
> confirmation takes quite a bit of scrolling to check with the function
> types.

Yeah, that is fair. Personally I prefer more descriptive names when
there are several or they have a special/asymmetric role.

> Otherwise I think perfect api docs.

Glad you like it!

> Just a quick comment on this, that's the same we do on the C side. Most
> overview chapters are actually DOC: sections pulled in from the code.
>
> What I meant here is that for big overview stuff (like for modesetting how
> the display pipe structures tie together as an example:
> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#overview)
> it doesn't make sense to duplicate that in rustdoc once more.

Yeah, definitely, if it is already somewhere else for C, we shouldn't
duplicate it (that is what I meant by the "shared across C and Rust"
exception).

> Maybe drm is the exception, but if you look at our .rst files we also have
> most of our docs in the code:
>
> https://cgit.freedesktop.org/drm/drm/tree/Documentation/gpu/drm-kms-helpers.rst
>
> The rst files just provide the scaffolding because C dosn't have
> crates/modules hierarchy that would do this for you automatically.

Sorry, I was talking in general in the kernel. That
`drm-kms-helpers.rst` looks great.

>From a quick grep, I think you are indeed one of the big users of
`DOC: `, which indeed map closely to what you would do in Rust without
the scaffolding need.

So I think you will like writing docs in Rust :)

Cheers,
Miguel




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux