Re: [Python-Dev] Anyone can do patch reviews (was: Enhanced tracker privileges...)
On Tue, Apr 27, 2010 at 02:40:19PM -, exar...@twistedmatrix.com wrote: This list would make a good addition to one of the cpython development pages. If potential contributors could find this information, then they'd be much more likely to participate by doing reviews. If anyone wants to write the updated text, the following command will make an anonymous read-only checkout of the /dev/ pages: svn ls https://svn.python.org/www/trunk/beta.python.org/build/data/dev I can apply a patch. --amk ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews (was: Enhanced tracker privileges...)
On Tue, 27 Apr 2010 11:15:49 +1000, Steven D'Aprano st...@pearwood.info wrote: No, of course not. There are always other reasons, the biggest is too many things to do and not enough time to do it. If I did review patches, would they be accepted on the strength on my untrusted reviews? It is very very helpful for *anyone* to review patches. Let's see if I can clarify the process a little. (This is, of course, my take on it, others can chime in if they think I got anything wrong.) Someone submits a bug. Someone submits a patch to fix that bug (or add the enhancement). Is that patch ready for commit? No. Is it ready for *commit review* (ie: someone with commit privileges to look at it with an eye toward committing it)? Probably not. What makes a patch ready for commit review? The patch should: 1) conform to pep 7/8 2) have unit tests that fail before the patch and succeed after 3) have documentation updates if needed 4) have a py3k port *if and only if* the port is non-trivial (well, if someone wants to add one when it is trivial that's OK, but it probably won't get used) 5) if it is at all likely to have system dependencies, be tested on at least linux and windows Making sure that these items are true does not require any in-depth expertise. In many cases it doesn't even require much time. 'Trusted' or 'untrusted' doesn't really come in to it, though doing these sorts of reviews will build trust. If you can in addition look at the patch content and critique it, so much the better. Again, *any* critique is useful, even if you can't review the whole patch in detail, because it gets it that much closer to being commit ready. And there are enough uncommitted patches in the tracker that it ought to be possible for almost anyone to find something they can usefully do a content critique on. The goal is to make the commit review step as simple and fast for the committer as possible. The more eyes on the patch before hand, the faster the commit review will be. And those people who do a good job making patches commit ready will be on the fast track to getting commit privileges. -- R. David Murray www.bitdance.com PS: note that I'm using 'commit review' above with a different sense than that value is currently defined to have in the workflow. I'm thinking about advocating that the definition in the workflow be changed, and indeed we (the informal triage crew) have already occasionally used that setting with the meaning I give it above. ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews (was: Enhanced tracker privileges...)
On 01:38 pm, rdmur...@bitdance.com wrote: On Tue, 27 Apr 2010 11:15:49 +1000, Steven D'Aprano st...@pearwood.info wrote: No, of course not. There are always other reasons, the biggest is too many things to do and not enough time to do it. If I did review patches, would they be accepted on the strength on my untrusted reviews? It is very very helpful for *anyone* to review patches. Let's see if I can clarify the process a little. (This is, of course, my take on it, others can chime in if they think I got anything wrong.) Someone submits a bug. Someone submits a patch to fix that bug (or add the enhancement). Is that patch ready for commit? No. Is it ready for *commit review* (ie: someone with commit privileges to look at it with an eye toward committing it)? Probably not. What makes a patch ready for commit review? The patch should: 1) conform to pep 7/8 2) have unit tests that fail before the patch and succeed after 3) have documentation updates if needed 4) have a py3k port *if and only if* the port is non-trivial (well, if someone wants to add one when it is trivial that's OK, but it probably won't get used) 5) if it is at all likely to have system dependencies, be tested on at least linux and windows This list would make a good addition to one of the cpython development pages. If potential contributors could find this information, then they'd be much more likely to participate by doing reviews. Jean-Paul ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews (was: Enhanced tracker privileges...)
On Apr 27, 2010, at 02:40 PM, exar...@twistedmatrix.com wrote:
On 01:38 pm, rdmur...@bitdance.com wrote:
2) have unit tests that fail before the patch and succeed after
This list would make a good addition to one of the cpython development
pages. If potential contributors could find this information, then
they'd be much more likely to participate by doing reviews.
It would be kind of cool if there were some best practices for running said
unittest both with and without the patch enabled. Kind of like using #ifdefs
in C but without all the commenting-out-commenting-in error proneness. I
guess you could do something like
if os.getenv('BUG1234'):
# Patch the frobnicator to not bloviate.
Maybe more trouble than it's worth, and not always feasible of course, but I'm
wondering how (or maybe if) people do things this way.
With Bazaar, I often use a loom with two threads - a bottom one that contains
the test that fails, and a top one that contains the fix for the test. It's a
great way to develop a patch, but you lose that once you flatten the code for
review.
-Barry
signature.asc
Description: PGP signature
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews (was: Enhanced tracker privileges...)
On Tue, 27 Apr 2010 11:16:51 -0400, Barry Warsaw ba...@python.org wrote:
It would be kind of cool if there were some best practices for running said
unittest both with and without the patch enabled. Kind of like using #ifdefs
in C but without all the commenting-out-commenting-in error proneness. I
guess you could do something like
if os.getenv('BUG1234'):
# Patch the frobnicator to not bloviate.
Maybe more trouble than it's worth, and not always feasible of course, but I'm
wondering how (or maybe if) people do things this way.
With Bazaar, I often use a loom with two threads - a bottom one that contains
the test that fails, and a top one that contains the fix for the test. It's a
great way to develop a patch, but you lose that once you flatten the code for
review.
Well, the way I do it for review is brute force: I download the patch,
delete everything except the unit test, apply that, run it, revert,
apply the original patch, run it.
For developing, I generally write the unit test first grin, but when
the fix is confined to one file I can just revert the file for testing
the tests while keeping the fixed copy in my edit buffer (or a save file
if I'm feeling paranoid, like when it is a substantial fix). For more
complex fixes I generate separate patch files for the tests and the fix
as a whole, and do a revert-patch-revert-patch dance to test things.
I wonder if it would be better to encourage people to post the unit
tests and the fix as separate patch files.
--
R. David Murray www.bitdance.com
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews (was: Enhanced tracker privileges...)
On Tue, Apr 27, 2010 at 5:16 PM, Barry Warsaw ba...@python.org wrote:
On Apr 27, 2010, at 02:40 PM, exar...@twistedmatrix.com wrote:
On 01:38 pm, rdmur...@bitdance.com wrote:
2) have unit tests that fail before the patch and succeed after
This list would make a good addition to one of the cpython development
pages. If potential contributors could find this information, then
they'd be much more likely to participate by doing reviews.
It would be kind of cool if there were some best practices for running said
unittest both with and without the patch enabled. Kind of like using
#ifdefs
in C but without all the commenting-out-commenting-in error proneness. I
guess you could do something like
if os.getenv('BUG1234'):
# Patch the frobnicator to not bloviate.
When I'm writing the patch it's usually easy, I write the tests, see that
they fail, write the fix, see that they pass.
When I'm reviewing the patch, I apply the patch, see that the tests pass,
svn revert the fix, check that they fail.
Most of the patches affect just a couple of files, so applying the whole
patch and then revert is usually trivial and probably easier than having to
deal with two separate files for patch and tests.
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews (was: Enhanced tracker privileges...)
On Apr 27, 2010, at 11:43 AM, R. David Murray wrote: I wonder if it would be better to encourage people to post the unit tests and the fix as separate patch files. I think this is not bad idea for larger fixes, where it's not trivial to manually edit the diff. -Barry signature.asc Description: PGP signature ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 R. David Murray wrote: On Tue, 27 Apr 2010 11:15:49 +1000, Steven D'Aprano st...@pearwood.info wrote: No, of course not. There are always other reasons, the biggest is too many things to do and not enough time to do it. If I did review patches, would they be accepted on the strength on my untrusted reviews? It is very very helpful for *anyone* to review patches. Let's see if I can clarify the process a little. (This is, of course, my take on it, others can chime in if they think I got anything wrong.) Someone submits a bug. Someone submits a patch to fix that bug (or add the enhancement). Is that patch ready for commit? No. Is it ready for *commit review* (ie: someone with commit privileges to look at it with an eye toward committing it)? Probably not. What makes a patch ready for commit review? The patch should: 1) conform to pep 7/8 2) have unit tests that fail before the patch and succeed after 3) have documentation updates if needed 4) have a py3k port *if and only if* the port is non-trivial (well, if someone wants to add one when it is trivial that's OK, but it probably won't get used) 5) if it is at all likely to have system dependencies, be tested on at least linux and windows This is an excellent set of guidelines. The only drawback I see here is that the current VCS situation makes doing the review more tedious than it should be, especially for non-committers. Or maybe the Hg mirrors are truly up-to-date and working? Last I looked, they were lagging or unavailable. Making sure that these items are true does not require any in-depth expertise. In many cases it doesn't even require much time. 'Trusted' or 'untrusted' doesn't really come in to it, though doing these sorts of reviews will build trust. If you can in addition look at the patch content and critique it, so much the better. Again, *any* critique is useful, even if you can't review the whole patch in detail, because it gets it that much closer to being commit ready. And there are enough uncommitted patches in the tracker that it ought to be possible for almost anyone to find something they can usefully do a content critique on. The goal is to make the commit review step as simple and fast for the committer as possible. The more eyes on the patch before hand, the faster the commit review will be. And those people who do a good job making patches commit ready will be on the fast track to getting commit privileges. -- R. David Murray www.bitdance.com PS: note that I'm using 'commit review' above with a different sense than that value is currently defined to have in the workflow. I'm thinking about advocating that the definition in the workflow be changed, and indeed we (the informal triage crew) have already occasionally used that setting with the meaning I give it above. Tres. - -- === Tres Seaver +1 540-429-0999 tsea...@palladion.com Palladion Software Excellence by Designhttp://palladion.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkvXLtkACgkQ+gerLs4ltQ79DACbB35/XFGyiYjd79OtTx+kgoNl mcsAnA4TNlM1ARjyrDrQIwv4KG48w/7h =1hGI -END PGP SIGNATURE- ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Barry Warsaw wrote:
On Apr 27, 2010, at 02:40 PM, exar...@twistedmatrix.com wrote:
On 01:38 pm, rdmur...@bitdance.com wrote:
2) have unit tests that fail before the patch and succeed after
This list would make a good addition to one of the cpython development
pages. If potential contributors could find this information, then
they'd be much more likely to participate by doing reviews.
It would be kind of cool if there were some best practices for running said
unittest both with and without the patch enabled. Kind of like using #ifdefs
in C but without all the commenting-out-commenting-in error proneness. I
guess you could do something like
if os.getenv('BUG1234'):
# Patch the frobnicator to not bloviate.
Maybe more trouble than it's worth, and not always feasible of course, but I'm
wondering how (or maybe if) people do things this way.
With Bazaar, I often use a loom with two threads - a bottom one that contains
the test that fails, and a top one that contains the fix for the test. It's a
great way to develop a patch, but you lose that once you flatten the code for
review.
You can always shelve the part of the patch which isn't the test: I
do that pretty frequently in the Zope tree, where I am now doing most
development with bzr.
Tres.
- --
===
Tres Seaver +1 540-429-0999 tsea...@palladion.com
Palladion Software Excellence by Designhttp://palladion.com
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iEYEARECAAYFAkvXLuMACgkQ+gerLs4ltQ5HBQCgw7kqJ52kPz+0cwNSpyVUkCFA
yQUAoLHJiYi+59Cc7BCeL46hA+Wygo66
=93vQ
-END PGP SIGNATURE-
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews
Tres Seaver tseaver at palladion.com writes: This is an excellent set of guidelines. The only drawback I see here is that the current VCS situation makes doing the review more tedious than it should be, especially for non-committers. Or maybe the Hg mirrors are truly up-to-date and working? Last I looked, they were lagging or unavailable. If you only a review a patch (rather than say maintain and evolve it), there's no point in using hg rather than SVN. However, the mirrors are most of the time alive and up-to-date (sync period is around 5 minutes): http://code.python.org/hg (perhaps you're thinking about http://hg.python.org/, which is an experimental conversion of the SVN repo, not really meant for daily use) Regards Antoine. ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Antoine Pitrou wrote: Tres Seaver tseaver at palladion.com writes: This is an excellent set of guidelines. The only drawback I see here is that the current VCS situation makes doing the review more tedious than it should be, especially for non-committers. Or maybe the Hg mirrors are truly up-to-date and working? Last I looked, they were lagging or unavailable. If you only a review a patch (rather than say maintain and evolve it), there's no point in using hg rather than SVN. Hmm, it feels exactly the other way around to me: working with the DVCS tools while reviewiing a patch allows me to be more productive (e.g., using 'bzr shelve' or the equivalent hg subcommand). Making a local branch / clone for each issue also feels more natural than working in a read-only SVN checkout. However, the mirrors are most of the time alive and up-to-date (sync period is around 5 minutes): http://code.python.org/hg That was the URL I was trying to work with: it has been a couple of months since I last tried, however. (perhaps you're thinking about http://hg.python.org/, which is an experimental conversion of the SVN repo, not really meant for daily use) Tres. - -- === Tres Seaver +1 540-429-0999 tsea...@palladion.com Palladion Software Excellence by Designhttp://palladion.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkvXOacACgkQ+gerLs4ltQ6ZTwCfQ96RYQ6h/zdMOnFUJU3MkSC1 +o8An2CqK7fbpiCM3gBWZuRReG46xv+U =iWug -END PGP SIGNATURE- ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews
On Tue, Apr 27, 2010 at 03:23:19PM -0400, Tres Seaver wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Antoine Pitrou wrote: Tres Seaver tseaver at palladion.com writes: This is an excellent set of guidelines. The only drawback I see here is that the current VCS situation makes doing the review more tedious than it should be, especially for non-committers. Or maybe the Hg mirrors are truly up-to-date and working? Last I looked, they were lagging or unavailable. If you only a review a patch (rather than say maintain and evolve it), there's no point in using hg rather than SVN. Hmm, it feels exactly the other way around to me: working with the DVCS tools while reviewiing a patch allows me to be more productive (e.g., using 'bzr shelve' or the equivalent hg subcommand). Making a local branch / clone for each issue also feels more natural than working in a read-only SVN checkout. +1. I find it to be an excellent way to muck around with patches and make my own changes / diffs / etc. for a review process. (Not that I do any Python reviews, note. But it's a great technique in general.) It's also fantastically simple and esay to interact with patches that are branches on someone's bitbucket or github repo; much better than uploading and downloading patch files while in the middle of a discussion. cheers, --titus -- C. Titus Brown, c...@msu.edu ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews
On Apr 27, 2010, at 02:37 PM, Tres Seaver wrote: You can always shelve the part of the patch which isn't the test: I do that pretty frequently in the Zope tree, where I am now doing most development with bzr. Yes definitely. bzr-loom just makes that much easier to manage. -Barry signature.asc Description: PGP signature ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews
Hmm, it feels exactly the other way around to me: working with the DVCS tools while reviewiing a patch allows me to be more productive (e.g., using 'bzr shelve' or the equivalent hg subcommand). Just try using Subversion for some time again, and you'll see that it is not difficult at all. Start with a clean sandbox, then apply the patch. If you want to go back to the state without patch, revert the sandbox, if you need it again, apply it again. It's really simple. Regards, Martin ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Martin v. Löwis wrote: Hmm, it feels exactly the other way around to me: working with the DVCS tools while reviewiing a patch allows me to be more productive (e.g., using 'bzr shelve' or the equivalent hg subcommand). Just try using Subversion for some time again, and you'll see that it is not difficult at all. Start with a clean sandbox, then apply the patch. If you want to go back to the state without patch, revert the sandbox, if you need it again, apply it again. It's really simple. I use Subversion daily as a committer on multiple projects. Keeping multiple readon-only checkouts around to evaluate patches is much clunkier than maintaining DVCS branches for the same purpose. As a non-committer, what I would miss most is the ability to make local commits. Features like shelve, log -p, etc., are aslo extremely useful when analyzing a patch. Tres. - -- === Tres Seaver +1 540-429-0999 tsea...@palladion.com Palladion Software Excellence by Designhttp://palladion.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkvXVV8ACgkQ+gerLs4ltQ7JqwCfVZPkm8/3XUuuzpm/N+08x2RI KWYAn004cLJS3poYZ/4BvSFOGzMpwNuC =j3lH -END PGP SIGNATURE- ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews
Ezio Melotti wrote: When I'm writing the patch it's usually easy, I write the tests, see that they fail, write the fix, see that they pass. When I'm reviewing the patch, I apply the patch, see that the tests pass, svn revert the fix, check that they fail. Most of the patches affect just a couple of files, so applying the whole patch and then revert is usually trivial and probably easier than having to deal with two separate files for patch and tests. This would be pretty close to my typical workflow as well (*looks at list of assigned bugs that hasn't moved in weeks* well, it is the workflow when I actually *do* some Python coding...) Cheers, Nick. -- Nick Coghlan | ncogh...@gmail.com | Brisbane, Australia --- ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Anyone can do patch reviews
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 27/04/10 17:16, Barry Warsaw wrote: It would be kind of cool if there were some best practices for running said unittest both with and without the patch enabled. Kind of like using #ifdefs in C but without all the commenting-out-commenting-in error proneness. I guess you could do something like Mercurial queues are very useful here. You apply/deapply patches with a single command: http://hgbook.red-bean.com/read/managing-change-with-mercurial-queues.html I am using python SVN mercurial mirror and MQ (Mercurial Queues) for development, waiting for the real thing (Mercurial native working). - -- Jesus Cea Avion _/_/ _/_/_/_/_/_/ j...@jcea.es - http://www.jcea.es/ _/_/_/_/ _/_/_/_/ _/_/ jabber / xmpp:j...@jabber.org _/_/_/_/ _/_/_/_/_/ . _/_/ _/_/_/_/ _/_/ _/_/ Things are not so easy _/_/ _/_/_/_/ _/_/_/_/ _/_/ My name is Dump, Core Dump _/_/_/_/_/_/ _/_/ _/_/ El amor es poner tu felicidad en la felicidad de otro - Leibniz -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQCVAwUBS9dofJlgi5GaxT1NAQIyAQP/avvYJxxlY4lr58nHbjsuoROz1rQi7RrR qd8G3grsS9NXlYbygw0rERJyg9UgjDhJrZbwYEPGJkxTIUd/Vcnw/fIB6J+xuLlY sRnmh0P6ILOFTHYoZZZ/hxtfdMiZxqiMHO3Pfs8uBc5bGC0f23cqiTOFY0+ze7mU 3vUIcljhuRE= =oyQb -END PGP SIGNATURE- ___ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
