Hi Alejandro, Thanks for the look! A few comments and questions. On Sun, May 26, 2024 at 12:36:58PM +0200, Alejandro Colomar wrote: > Hi Brian, > > On Fri, May 24, 2024 at 12:08:28PM GMT, Brian Norris wrote: > > --- a/man/man2/sched_setattr.2 > > +++ b/man/man2/sched_setattr.2 > > u64 sched_runtime; > > u64 sched_deadline; > > u64 sched_period; > > + > > Please don't use blank lines in the source code. They trigger a > warning. Oops, I probably should have gotten further into the documentation to figure out how to run the linters. Indeed I see the warning now, and I'll make sure I don't add more lint in the next version. > > +These flags indicate that the > > +.I > > +sched_util_min > > +or > > +.I > > +sched_util_max > > +fields, respectively, are present, representing the expected minimum and > > +maximum utilization of the thread. > > Please use semantic newlines. > > $ MANWIDTH=72 man man-pages | sed -n '/Use semantic newlines/,/^$/p' I'll give that man page a better read for my next submission. Thanks for the callout. > Use semantic newlines > In the source of a manual page, new sentences should be started on > new lines, long sentences should be split into lines at clause > breaks (commas, semicolons, colons, and so on), and long clauses > should be split at phrase boundaries. This convention, sometimes > known as "semantic newlines", makes it easier to see the effect of > patches, which often operate at the level of individual sentences, > clauses, or phrases. I'll do my best to interpret what the best "phrase boundaries" are. I don't think the writing always has enough punctuation breaks to nicely break into 80-char pieces. > > @@ -353,7 +398,6 @@ .SH ERRORS > > .I attr.sched_flags > > contains a flag other than > > .BR SCHED_FLAG_RESET_ON_FORK ; > > -or > > This change seems to be unrelated to this patch, right? I suppose it's unrelated. At first I was going to add new EINVAL descriptions to this paragraph, and I found that it had an odd (incorrect?) use of too many "or". But then I simply broke out an additional EINVAL section, which makes this change less related. Side note: on second thought, it probably makes sense to split this paragraph into multiple anyway, since the pattern "condition A; or condition B; or condition C [...]" gets a bit hard to read with sufficient number of different conditions. If it's preferred (and based on your comment, it probably is?), I'll make corrections in separate patches. > > .I attr.sched_priority > > is invalid; or > > .I attr.sched_policy Regards, Brian
