On 29.05.2012 0:07, Brad Roberts wrote:
There's an analogy that we like to use at work, and in my experience it holds
pretty well for code quality and bleeds
pretty well into the entire development process:
http://en.wikipedia.org/wiki/Broken_windows_theory
We do an ok job when it comes to the tests (though certainly not perfect).
We've been getting a lot better at
addressing regressions, though there's still 4 open right now (1 phobos, 2
druntime, 1 dmd). Can we please make the
remaining open regressions a release blocker?
Also, I'd really love to see the pull lists knocked down considerably before
the next beta, particularly the phobos set
(considering the larger set of people with access to it). The d-p-l and tools
sets should be even easier to keep at 0,
but even those have some accumulated requests, the oldest of which is over a
year now.
Several of the open requests (across all the repos) are open because there's
some controversy about them. I'll pick the
'dur' ones as a good, but hardly only, example. Either those should be pulled
or closed. Leaving in limbo helps no
one. For this sort of request, a choice needs to be made and move on. A key
part of the problem with this set is that
we don't have a process for deciding what to do with them. Anyone care to put
forth a proposal?
Would be awesome to have a simple official way of handling pull request.
Throwing in something to start a discussion.
The D Pull Process (TDPP).
1. Pulls that are open longer then 1-2 weeks must be reviewed by at
least one core dev. (exceptions are large pulls, e.g. 1KLOC+ pulls)
2. The review MUST end with one of following verdicts (it should be
obvious, like separate message, no free form only exact terms*) :
a) "Good to go", followed by merge.
b) "Needs work", review prolonged by another 1-2 weeks to let
author address issues, then processed simillarly.
c) "Not good enough" or "Rewrite" means that idea is OK, but
implementation is no good and won't be accepted. Pull is either closed
or in select cases it can be kept so that contributor can rewrite it
with push --force.
d) "Rejected", followed by explanation a-la:
d.1 unfit, doesn't cover the problem set it objectively should
d.2 disruptive, breaks (too much of) existing code
d.3 pointless, adds no or too little value (observe that d.1 is
"misses the boat", here it's too small a benefit )
d.4 ineffective, (though that may be rewrite) if contributor
refuses to rewrite ;)
d.5 insecure/unsafe, breaking language or OS guarantees (proof
links may help revert this decision)
...
e) "Postponed", followed by close. Meaning that it delayed for
arbitrary long amount of time (not 1 month, but like "try again later")
Read it like "till we have x64 dmd on win32" ;)
3. Longest period of review (given few prolongs) is 3** months. Then
it's ether closed or merged, any possible limbo is handled via "e" -
postponed (e.g. no response form authors etc.).
*Names are placeholders and debatable.
**values too, obviously
Another point - it looks like core developers are hesitant to pull in
various stuff that touch parts of Phobos they know nothing about. Would
be great if we had coverage of factor ~2 at least. That is every module
in Phobos is known at least to 2 developers (known in general).
--
Dmitry Olshansky
_______________________________________________
phobos mailing list
[email protected]
http://lists.puremagic.com/mailman/listinfo/phobos
