Re: [PATCH] Documentation: add initial iomap kdoc

[Dave Chinner <david@fromorbit.com>]

On Fri, May 19, 2023 at 08:13:50AM -0700, Randy Dunlap wrote:
> 
> 
> On 5/19/23 02:28, Bagas Sanjaya wrote:
> >> +/**
> >> + * DOC:  Flags reported by the file system from iomap_begin
> >>   *
> >> - * IOMAP_F_NEW indicates that the blocks have been newly allocated and need
> >> - * zeroing for areas that no data is copied to.
> >> + * * IOMAP_F_NEW: indicates that the blocks have been newly allocated and need
> >> + *	zeroing for areas that no data is copied to.
> >>   *
> >> - * IOMAP_F_DIRTY indicates the inode has uncommitted metadata needed to access
> >> - * written data and requires fdatasync to commit them to persistent storage.
> >> - * This needs to take into account metadata changes that *may* be made at IO
> >> - * completion, such as file size updates from direct IO.
> >> + * * IOMAP_F_DIRTY: indicates the inode has uncommitted metadata needed to access
> >> + *	written data and requires fdatasync to commit them to persistent storage.
> >> + *	This needs to take into account metadata changes that *may* be made at IO
> >> + *	completion, such as file size updates from direct IO.
> >>   *
> >> - * IOMAP_F_SHARED indicates that the blocks are shared, and will need to be
> >> - * unshared as part a write.
> >> + * * IOMAP_F_SHARED: indicates that the blocks are shared, and will need to be
> >> + *	unshared as part a write.
> >>   *
> >> - * IOMAP_F_MERGED indicates that the iomap contains the merge of multiple block
> >> - * mappings.
> >> + * * IOMAP_F_MERGED: indicates that the iomap contains the merge of multiple block
> >> + *	mappings.
> >>   *
> >> - * IOMAP_F_BUFFER_HEAD indicates that the file system requires the use of
> >> - * buffer heads for this mapping.
> >> + * * IOMAP_F_BUFFER_HEAD: indicates that the file system requires the use of
> >> + *	buffer heads for this mapping.
> >>   *
> >> - * IOMAP_F_XATTR indicates that the iomap is for an extended attribute extent
> >> - * rather than a file data extent.
> >> + * * IOMAP_F_XATTR: indicates that the iomap is for an extended attribute extent
> >> + *	rather than a file data extent.
> >>   */
> > Why don't use kernel-doc comments to describe flags?
> > 
> 
> Because kernel-doc handles functions, structs, unions, and enums.
> Not defines.

So perhaps that should be fixed first?

I seriously dislike the implication here that we should accept
poorly/inconsistently written comments and code just to work around
deficiencies in documentation tooling.

Either modify the code to work cleanly and consistently with the
tooling (e.g. change the code to use enums rather than #defines), or
fix the tools that don't work with macro definitions in a way that
matches the existing code documentation standards.

Forcing developers, reviewers and maintainers to understand, accept
and then maintain inconsistent crap in the code just because some
tool they never use is deficient is pretty much my definition of an
unacceptible engineering process.

-Dave.
-- 
Dave Chinner
david@fromorbit.com