This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: -Werror policy


> If we let things through without comments, I think they're just as likely 
> to go through with unhelpful or empty strings.  Hence my proposed patch 
> having an unused argument only for a case where having some information 
> inside the macro call rather than elsewhere nearby seems useful (GCC 
> version with which the warning was observed, with a view to grepping for 
> cases needing review).

I don't really agree.  As a contributor, anyone is somewhat likely to
innocently overlook a policy requirement about comments.  As a reviewer,
months from now, I am pretty likely to see someone's change and think,
"That's simple enough and obvious enough" and just approve it without
noticing that the change was of a sort that has a specific policy
requirement about comments--even when I was the big stickler insisting that
we have that policy in the first place!  Conversely, when there is a
syntactic requirement to fill in a slot with something, then the
contributor is more likely to go and figure out what they were supposed to
put there.  We expect contributors to be conscientious and well-meaning, so
they won't shirk a policy obligation intentionally, just through oversight.
Moreover, as a reviewer, I will notice the slot where the explanation is
supposed to be because an empty macro argument or a stubby-looking string
will stick out like a sore thumb and make me remember the whole issue about
the comments policy so I push back and insist on following the policy.  I
can easily believe that you as a reviewer will never fail to demand the
commentary that a previously agreed-upon policy requires.  But I am quite
sure that I as a reviewer will fail some proportion of the time.


Thanks,
Roland


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]