From: Peter Zijlstra <peterz@infradead.org>
To: "Ahmed S. Darwish" <a.darwish@linutronix.de>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
"Paul E. McKenney" <paulmck@kernel.org>,
"Sebastian A. Siewior" <bigeasy@linutronix.de>,
Steven Rostedt <rostedt@goodmis.org>,
LKML <linux-kernel@vger.kernel.org>,
Jonathan Corbet <corbet@lwn.net>,
linux-doc@vger.kernel.org
Subject: Re: [PATCH v1 10/25] seqlock: Add RST directives to kernel-doc code samples and notes
Date: Mon, 25 May 2020 15:44:29 +0200 [thread overview]
Message-ID: <20200525134429.GE317569@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20200525093649.GA370823@debian-buster-darwi.lab.linutronix.de>
On Mon, May 25, 2020 at 11:36:49AM +0200, Ahmed S. Darwish wrote:
> Peter Zijlstra <peterz@infradead.org> wrote:
> > I will not let sensible code comments deteriorate to the benefit of some
> > external piece of crap.
> >
> > As a programmer the primary interface to all this is a text editor, not
> > a web broswer or a pdf file or whatever other bullshit.
> >
> > If comments are unreadable in your text editor, they're useless.
>
> Wait.
>
> Most of the patch in question is just substituting the code snippet's
> leading white spaces to tabs. For illustration purposes, if we remove
> these white space hunks from the diff, it becomes:
>
> --- a/include/linux/seqlock.h
> +++ b/include/linux/seqlock.h
> @@ -232,6 +232,8 @@ static inline void raw_write_seqcount_end(seqcount_t *s)
> + * .. code-block:: c
> ...
> + * .. code-block:: c
> ...
> - * NOTE: The non-requirement for atomic modifications does _NOT_ include
> - * the publishing of new entries in the case where data is a dynamic
> - * data structure.
> + * .. attention::
> + *
> + * The non-requirement for atomic modifications does _NOT_ include
> + * the publishing of new entries in the case where data is a dynamic
> + * data structure.
> ...
>
> Are you trying to tell me that, good heavens, these directives are
> really hurting your eyes so much?
Yep, they're a distraction and serve absolutely no purpose. They're also
utterly moronic, of course it's code and of course it's bloody well C.
> Putting kernel-doc aside... That huge raw_write_seqcount_latch() comment
> is actually *way more readable from any text editor* after applying this
> patch. Go figure.
I don't mind the re-indent.
> >>> The correct fix is, as always, to remove the kernel-doc marker.
>
> Sorry, that's not the correct fix.
Of course it is, if kerneldoc complains that a perfectly good comment
is no good, then the fault lies with kerneldoc.
It's like checkpatch; assume it is wrong :-)
> In the following patches, kernel-doc for the entire seqlock.h API is
> added. Singling out raw_write_seqcount_latch() doesn't make any sense.
% s/\/\*\*/\/\*/g -- tada!!
> If you look at the top of this patch series, a lot of seqlock.h
> seqcount_t call sites were badly broken. The 0day kernel test bot sent
> me even more erroneous call sites due to the added lockdep checks. This
> is an extra argument for the added documentation: the existing one is
> horrible.
I've nothing against improving comments, I'm just saying that RST is
absolute atrocious shite and has nothing to do with good comments.
If sphinx doesn't like "NOTE:' when go teach it.
> So, please, don't claim that the current situation is fine. It is not.
I've never claimed that. My claim is that RST is shite and has no added
value.
next prev parent reply other threads:[~2020-05-25 13:44 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-19 21:45 [PATCH v1 00/25] seqlock: Extend seqcount API with associated locks Ahmed S. Darwish
2020-05-19 21:45 ` [PATCH v1 09/25] Documentation: locking: Describe seqlock design and usage Ahmed S. Darwish
2020-05-22 18:01 ` Peter Zijlstra
2020-05-22 22:24 ` Steven Rostedt
2020-05-25 10:50 ` Ahmed S. Darwish
2020-05-25 11:02 ` Ahmed S. Darwish
2020-05-19 21:45 ` [PATCH v1 10/25] seqlock: Add RST directives to kernel-doc code samples and notes Ahmed S. Darwish
2020-05-22 18:02 ` Peter Zijlstra
2020-05-22 18:03 ` Peter Zijlstra
2020-05-22 18:26 ` Thomas Gleixner
2020-05-22 18:32 ` Peter Zijlstra
2020-05-25 9:36 ` Ahmed S. Darwish
2020-05-25 13:44 ` Peter Zijlstra [this message]
2020-05-25 14:07 ` Peter Zijlstra
2020-05-19 21:45 ` [PATCH v1 11/25] seqlock: Add missing kernel-doc annotations Ahmed S. Darwish
2020-05-19 21:45 ` [PATCH v1 12/25] seqlock: Extend seqcount API with associated locks Ahmed S. Darwish
2020-06-08 0:57 ` [PATCH v2 00/18] " Ahmed S. Darwish
2020-06-08 0:57 ` [PATCH v2 01/18] Documentation: locking: Describe seqlock design and usage Ahmed S. Darwish
2020-06-08 0:57 ` [PATCH v2 02/18] seqlock: Properly format kernel-doc code samples Ahmed S. Darwish
2020-06-08 0:57 ` [PATCH v2 03/18] seqlock: Add missing kernel-doc annotations Ahmed S. Darwish
2020-06-30 5:44 ` [PATCH v3 00/20] seqlock: Extend seqcount API with associated locks Ahmed S. Darwish
2020-06-30 5:44 ` [PATCH v3 01/20] Documentation: locking: Describe seqlock design and usage Ahmed S. Darwish
2020-07-06 21:04 ` Peter Zijlstra
2020-07-06 21:12 ` Jonathan Corbet
2020-07-06 21:16 ` Peter Zijlstra
2020-07-07 10:12 ` Ahmed S. Darwish
2020-07-07 12:47 ` Peter Zijlstra
2020-06-30 5:44 ` [PATCH v3 02/20] seqlock: Properly format kernel-doc code samples Ahmed S. Darwish
2020-06-30 5:44 ` [PATCH v3 03/20] seqlock: Add missing kernel-doc annotations Ahmed S. Darwish
2020-07-20 15:55 ` [PATCH v4 00/24] seqlock: Extend seqcount API with associated locks Ahmed S. Darwish
2020-07-20 15:55 ` [PATCH v4 01/24] Documentation: locking: Describe seqlock design and usage Ahmed S. Darwish
2020-07-21 1:35 ` Steven Rostedt
2020-07-21 1:37 ` Steven Rostedt
2020-07-21 5:34 ` Ahmed S. Darwish
2020-07-21 1:44 ` Steven Rostedt
2020-07-21 1:51 ` Steven Rostedt
2020-07-21 7:15 ` Ahmed S. Darwish
2020-07-20 15:55 ` [PATCH v4 02/24] seqlock: Properly format kernel-doc code samples Ahmed S. Darwish
2020-07-20 15:55 ` [PATCH v4 03/24] seqlock: seqcount_t latch: End read sections with read_seqcount_retry() Ahmed S. Darwish
2020-07-20 15:55 ` [PATCH v4 05/24] seqlock: Add kernel-doc for seqcount_t and seqlock_t APIs Ahmed S. Darwish
2020-07-20 16:49 ` [PATCH v4 00/24] seqlock: Extend seqcount API with associated locks Eric Biggers
2020-07-20 17:33 ` Ahmed S. Darwish
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200525134429.GE317569@hirez.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=a.darwish@linutronix.de \
--cc=bigeasy@linutronix.de \
--cc=corbet@lwn.net \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=paulmck@kernel.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).