develooper Front page | perl.perl5.porters | Postings from July 2013

Re: [perl #116296] [PATCH] remove PL_patchlevel from and optimizeS_minus_v

Thread Previous | Thread Next
Ricardo Signes
July 9, 2013 22:59
Re: [perl #116296] [PATCH] remove PL_patchlevel from and optimizeS_minus_v
Message ID:
* bulk88 via RT <> [2013-07-08T18:36:35]
> When I hear "your favorites aren't my favorites so burn in hell", I see
> that commenter is wearing rose glasses and it is political/religious BS.
> [ ⋮ ]
> The same opinions are coming out again, self-righteous "if I wouldn't
> change it, you have no right to either"

I think that this is not only hyperbole (which is one thing), but also unfair
and, possibly worst of all, not likely to convince anybody of anything helpful.

In every case that I've seen, the issue has not been "I don't care about your
compiler," but "this optimization is only an optimization sometimes, and other
times a cost or no change, but carries a code complexity cost."

> Commiters are held to no standard of code clarity or documentation. I've
> never seen a revert commit to revert something for lack of comments or
> clarity.

I encourage you to politely point out such shortcomings as they occur, to
remind other contributors to improve their comments post facto and to improve
their commit messages next time.

I think it would be in some ways nice, and would have some positive effects, to
have a policy stating that a commiter may not, barring exception circumstances,
apply his own code to blead, but must have a second set of eyes in the form of
another committer who applies the author's branch.  Unfortunately, I think that
this is going to be a hard sell, at best.

We need a better culture of code review, I agree.  I also agree that it would
benefit everyone were that code review applied not only to code coming from
non-committers, but also within the group of committers.  That said, I don't,
of course, think the solution is to have no standards at all.

That's why I hope you will, in the future, politely point out when commits have
been made that are of unclear purpose or are under-explained.  If the commits
are on a branch before merging and you have seen it, all the better, so it can
be fixed before it hits blead.  If not, it's something for the author to keep
in mind for next time.


Thread Previous | Thread Next Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About