improving our code quality [my summary of the "PQM" thread]
The thread on PQM has now grown multiple points and it becoming a little hard to follow since it is pulling in a bunch of ideas not directly related to a PQM system, so I am going to attempt to summarize the points brought up here. SUMMARY: it's great we are talking about this, but I think this would be better to discuss after 2.6/3.0 is out the door.
What are our dev rules, and are we following them?
Obviously this is all pointing out the fact that Python's popularity has grown such that the amount of hours available from core developers is far exceeded by what we really need to keep stuff going the way we have been doing things. Short of tossing out commit privileges like candy on Halloween outside of a grade school, something really should change.
We already have some things in place that do help. Requiring tests has helped catch issues on the buildbots and made sure we didn't repeat the same mistakes. We also all know better than to introduce anything major during the beta cycle, but the thought of having some solution for multi-core processing was too tempting so we ignored general practice for the multiprocessing module and now we are paying for it late in the release cycle. We also want everyone to run the ENTIRE testing suite before a commit is made, but I am sure we are all guilty of skipping a full '-u all' run when the change is thought to be minor (and I know I never run with random test order like the buildbots).
Code reviews
One suggestion on how to improve the situation has been code reviews. Ideas have ranged from code reviews for everything to "who needs code reviews?" Doing a code review on everything will obviously lower the bandwidth on committed code and we already don't have that many people who review pre-existing patches as it is. But some review should probably be done for anything significant. I am leery of suggesting we go with guidelines based on line count, but probably anything that takes more than an afternoon to implement should probably get a code review. And yes, working in branches would help with this.
Buildbots
We also have the buildbots which are handy when you are trying to fix something that is multi-platform, but otherwise they are just email generators. I think we really need to work on fix the various flaky tests so that false-positive failures stop occurring and leading to people ignoring the buildbot emails. And I personally like Christian's idea of a single summary email at the end of the day of buildbots that are still failing that goes to everyone potentially involved in the breakage along with to a mailing list.
PQM
There is also the PQM suggestion to make sure we always at least have some general sanity in the code base. But a PQM system would probably only work once the test suite is not flaky anymore as having some typo in a comment be rejected because test_kqueue failed again is not going to go over well with most people. And that also goes for OS-specific failures on a platform I am not running on. Honestly the PQM system probably wouldn't be necessary if people just ran the entire test suite before a checkin. And if people never checked in if there was any failure (related to their code or not) we would probably get the test suite cleaned up a lot faster as well.
How are we going to solve this?
I think a thorough discussion on how we have been handling development is needed. We all want to continue to enjoy developing for Python and we want it to continue to be a high-quality project. Because of this we might have to compromise on what we might do if this were a business with paid positions, but I don't think it will be anything major. And I think the 2.6/3.0 development cycle has shown we really should be changing something to make our lives easier.
But I honestly think this discussion should wait until after we go final with 2.6/3.0. Once that happens and we are all not running around trying to get a release out, then we should start from the ground up and re-evaluate what we need to change. That includes the workflow in the issue tracker, how the buildbots are used, what our expected best practices are, cleaning up the test suite, how serious we are about moving to a distributed VCS, whether we want a PQM, do we want code reviews, etc. But I really think we should be putting the time in now to b3 and working towards hitting final before we delve into this lengthy conversation.
And yes, I am willing to help help with all of this through PEPs, documenting what are guidelines are for new developers, doing stuff through the infrastructure committee, etc. (although importlib will be finished first, although not necessarily committed if our commit practices change, since I had to miss 3.0 in order to get PEP 3108 done and I refuse to miss another release).
-Brett
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On Aug 14, 2008, at 2:06 PM, Brett Cannon wrote:
But I honestly think this discussion should wait until after we go final with 2.6/3.0. Once that happens and we are all not running around trying to get a release out, then we should start from the ground up and re-evaluate what we need to change. That includes the workflow in the issue tracker, how the buildbots are used, what our expected best practices are, cleaning up the test suite, how serious we are about moving to a distributed VCS, whether we want a PQM, do we want code reviews, etc. But I really think we should be putting the time in now to b3 and working towards hitting final before we delve into this lengthy conversation.
Thanks for the nice summary Brett. I completely agree with you that
any changes in process should happen after 2.6/3.0. I'll now go back
to harassing people old skool. :)
- -Barry
-----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (Darwin)
iQCVAwUBSKR3lXEjvBPtnXfVAQLA2AQAkQZHoL4YH8Di+hKzq3nVdJXazSJPL1Va 5xFnobMmrjKYnvfIK1N50Vg/9pSzxLXVvU1OMYsJdoUnXq1NXaleBIfJbjdMtck5 9CULBncXnokD6IioUI410xhJIXsn/39Xw2INVGiO9GwHy9Y9NlkaEs7xvh8uOkP0 yC8P8+Phu6s= =/bFZ -----END PGP SIGNATURE-----
Hey Brett,
There is also the PQM suggestion to make sure we always at least have some general sanity in the code base. But a PQM system would probably only work once the test suite is not flaky anymore as having some typo in a comment be rejected because test_kqueue failed again is not going to go over well with most people.
That's true, but these tests should actually be disabled or fixed. There's no point in having a test that everyone finds "ok" to have it broken.
And that also goes for OS-specific failures on a platform I am not running on. Honestly the PQM system probably wouldn't be necessary if people just ran the entire test suite before a checkin. And if
The reason to use PQM is precisely so that your commit *does* break when you create breakage in a platform you're not running on. Even though you might not care about breaking someone else's environment, I believe it's in the best interest of everyone that this doesn't ever happen in the main line of development.
Note that I'm playing devil's advocate here. Just like everyone else, I do enjoy being able to commit straight to the repository and seeing my changes there. On the other hand, in a complex environment where several developers and multiple platforms are involved, the only way to bring constant stability in is to penalize those that cause breakage.
-- Gustavo Niemeyer http://niemeyer.net
2008/8/14 Brett Cannon <brett@python.org>:
But I honestly think this discussion should wait until after we go final with 2.6/3.0. Once that happens and we are all not running
Thanks Brett for this. I started to write some responses... and then I realized I agree with you here, it's better to wait for the release, and then rethink some processes.
around trying to get a release out, then we should start from the ground up and re-evaluate what we need to change. That includes the workflow in the issue tracker, how the buildbots are used, what our expected best practices are, cleaning up the test suite, how serious we are about moving to a distributed VCS, whether we want a PQM, do we want code reviews, etc. But I really think we should be putting the
Don't forget the issues tracker. I think it's an awesome tool (the bug tracking in general, roundup in particular), but I think it could be improved.
In particular, I hate the state of some bugs where some discussion is in place, a decission needs to be taken, but the discussion fades out, nobody takes that decision (normally because the people thinks that they don't know enough of that domain), and then the bug just stalls. And, if you pick up that bug six month later, or three year later, you lose twenty minutes reading the whole discussion, and then realize that neither you have the expertise to get a decision, and skip to the next bug.
In the last Python Bug Day, in Argentina this happened a lot: people digged and digged through the bugs, losing one or two hours before they get to something solvable (but yet not so easy).
Don't know how to solve this, and I'm pretty sure I'm not pointing out all the problems: just don't forget to include this in the list.
Thanks again!
-- . Facundo
Blog: http://www.taniquetil.com.ar/plog/ PyAr: http://www.python.org/ar/
On Thu, Aug 14, 2008 at 11:48:49PM -0300, Facundo Batista wrote:
And, if you pick up that bug six month later, or three year later, you lose twenty minutes reading the whole discussion, and then realize that neither you have the expertise to get a decision, and skip to the next bug. ... Don't know how to solve this, and I'm pretty sure I'm not pointing out all the problems: just don't forget to include this in the list.
Should we add a 'needsreview' or 'ready-for-review' keyword that could be marked on such bugs? People could check for it before diving into a bug, and the mythical reviewer could use it too.
--amk
On Aug 15, 2008, at 10:54 AM, "A.M. Kuchling" <amk@amk.ca> wrote:
On Thu, Aug 14, 2008 at 11:48:49PM -0300, Facundo Batista wrote:
And, if you pick up that bug six month later, or three year later,
you lose twenty minutes reading the whole discussion, and then realize that neither you have the expertise to get a decision, and skip to
the next bug. ... Don't know how to solve this, and I'm pretty sure I'm not pointing
out all the problems: just don't forget to include this in the list.Should we add a 'needsreview' or 'ready-for-review' keyword that could be marked on such bugs? People could check for it before diving into a bug, and the mythical reviewer could use it too.
Would it be possible / make sense to tie this more tightly to the code
review application guido put together?
Perhaps a patch set to needsreview gets an automatic ticket / upload
to the codereview app?
I'm just thinking that we have a good code review app and a good
ticket system, maybe we just need to use the code review system more
-Jesse
Jesse> Would it be possible / make sense to tie this more tightly to the
Jesse> code review application guido put together?
Drifting a bit far afield, but are we using Guido's tool (Rietveld)? If not, maybe the BDFL should put his BBDF (Big Benevolent Dictator's Foot) down and decree no more releases until it's part of the Python development tool chain. <0.5 wink>
Skip
Jesse> Would it be possible / make sense to tie this more tightly to the Jesse> code review application guido put together?
Drifting a bit far afield, but are we using Guido's tool (Rietveld)? If not, maybe the BDFL should put his BBDF (Big Benevolent Dictator's Foot) down and decree no more releases until it's part of the Python development tool chain. <0.5 wink>
I would still like to hear from someone who's used both Rietveld and the older (and from what I can tell more widely used) Review Board.
</F>
Fredrik> I would still like to hear from someone who's used both
Fredrik> Rietveld and the older (and from what I can tell more widely
Fredrik> used) Review Board.
Good point. A bit to Python-focused in my note.
Skip
I'm just thinking that we have a good code review app and a good ticket system, maybe we just need to use the code review system more
Rietveld will already send reviews into the bug tracker, assuming you make the bug tracker a reviewer, or otherwise a message recipient.
Regards, Martin
2008/8/15 A.M. Kuchling <amk@amk.ca>:
Should we add a 'needsreview' or 'ready-for-review' keyword that could be marked on such bugs? People could check for it before diving into a bug, and the mythical reviewer could use it too.
*Maybe*. It'd be the fourth keyword there.
But I don't want to start populating that option, and reach the same issue we have now with "Resolution", without fixing first that issue.
(I'm deliberately trying to avoid getting into how to fix that, we should leave this for after the releases...)
Regards,
-- . Facundo
Blog: http://www.taniquetil.com.ar/plog/ PyAr: http://www.python.org/ar/
participants (9)
-
"Martin v. Löwis"
-
A.M. Kuchling
-
Barry Warsaw
-
Brett Cannon
-
Facundo Batista
-
Fredrik Lundh
-
Gustavo Niemeyer
-
Jesse Noller
-
skip@pobox.com