Rietveld integration into Roundup
Following up to the recent thread, I have now integrated Rietveld into Roundup. This is a rough draft still, and highly experimental. Please try it out, but expect that it may be necessary to discard all data (including comments) to start over (of course, I'll try to avoid having to do so).
The Rietveld integration currently works this way: - the installation lives at http://bugs.python.org/review/ - for each roundup user, a rietveld user is created; log into roundup to get access to rietveld - for each roundup issue, a rietveld issue is created with the same issue id. Please don't create additional Rietveld issues. - regularly (currently every hour), each patch is considered. Patches are skipped if: * they have been added to Rietveld already * they have no clear base version (i.e. they don't originate from svn diff) * they belong to no or a closed issue * they apply to a patch that is not currently supported (only trunk, 2.6, 2.7, 3.1, py3k are currently supported) - for each such patch, a patchset is created - if that is successful, a "review" link is added to roundup
Feel free to discuss this here; bug reports and feature requests should go to the meta tracker. Contributions are welcome; I won't be able to work on this much for the next four days.
Regards, Martin
Thanks for this. It looks very nice.
2010/10/2 "Martin v. Löwis" martin@v.loewis.de:
Following up to the recent thread, I have now integrated Rietveld into Roundup. This is a rough draft still, and highly experimental. Please try it out, but expect that it may be necessary to discard all data (including comments) to start over (of course, I'll try to avoid having to do so).
The Rietveld integration currently works this way:
- the installation lives at http://bugs.python.org/review/
- for each roundup user, a rietveld user is created;
log into roundup to get access to rietveld
- for each roundup issue, a rietveld issue is created with the
same issue id. Please don't create additional Rietveld issues.
- regularly (currently every hour), each patch is considered.
Patches are skipped if: * they have been added to Rietveld already * they have no clear base version (i.e. they don't originate from svn diff) * they belong to no or a closed issue * they apply to a patch that is not currently supported (only trunk, 2.6, 2.7, 3.1, py3k are currently supported)
- for each such patch, a patchset is created
- if that is successful, a "review" link is added to roundup
Feel free to discuss this here; bug reports and feature requests should go to the meta tracker. Contributions are welcome; I won't be able to work on this much for the next four days.
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/g.rodola%40gmail.com
Hello,
On Sat, 02 Oct 2010 22:03:57 +0200 "Martin v. Löwis" martin@v.loewis.de wrote:
Following up to the recent thread, I have now integrated Rietveld into Roundup. This is a rough draft still, and highly experimental. Please try it out, but expect that it may be necessary to discard all data (including comments) to start over (of course, I'll try to avoid having to do so).
Thank you! This looks promising.
I have some questions:
1) who is the notification e-mail sent to when a review is published? I just tried on one of my patches and the e-mail came with the following headers:
From: pitrou@free.fr Reply-to: pitrou@free.fr, None@psf.upfronthosting.co.za To: pitrou@free.fr Cc: None@psf.upfronthosting.co.za Sujet: Fast path for small int-indexing of lists and tuples (issue9800)
Am I right in assuming that nobody except me (even other people nosied in the Roundup issue) received the notification?
(is None@psf some kind of /dev/null?)
2) if I look at http://bugs.python.org/issue9962, only the second patch of all three has been enabled for review. Yet they were all created through "svn diff" against a recent py3k checkout.
Thanks again
Antoine.
- who is the notification e-mail sent to when a review is published?
I haven't figured that out yet. I'm not even sure whether email gets sent at all. I'll look into this a week from now, unless someone resolves that earlier. But...
I just tried on one of my patches and the e-mail came with the following headers:
From: pitrou@free.fr Reply-to: pitrou@free.fr, None@psf.upfronthosting.co.za To: pitrou@free.fr Cc: None@psf.upfronthosting.co.za Sujet: Fast path for small int-indexing of lists and tuples (issue9800)
Am I right in assuming that nobody except me (even other people nosied in the Roundup issue) received the notification?
So email did get send ... good.
It certainly doesn't consider the nosy list at all at the moment.
(is None@psf some kind of /dev/null?)
I have no idea where that came from.
- if I look at http://bugs.python.org/issue9962, only the second patch
of all three has been enabled for review. Yet they were all created through "svn diff" against a recent py3k checkout.
They had both the same problem: it could figure out the revision number the patch applied to (e.g. 85039), but not the branch that this revision is for. That's because the revision is 85039, and in r85039, only /peps got modified.
I see... so the revision number is mostly useless when trying to identify the branch.
You should be able to fix this by manually filling out the branch on the file. I'll have to come up with a better way to determine the branch which a patch was created on. Perhaps going back in history and taking the first branch where the patch cleanly applies can do the trick.
Regards, Martin
Le samedi 02 octobre 2010 à 22:55 +0200, "Martin v. Löwis" a écrit :
- if I look at http://bugs.python.org/issue9962, only the second patch
of all three has been enabled for review. Yet they were all created through "svn diff" against a recent py3k checkout.
They had both the same problem: it could figure out the revision number the patch applied to (e.g. 85039), but not the branch that this revision is for. That's because the revision is 85039, and in r85039, only /peps got modified.
I see... so the revision number is mostly useless when trying to identify the branch.
You should be able to fix this by manually filling out the branch on the file. I'll have to come up with a better way to determine the branch which a patch was created on. Perhaps going back in history and taking the first branch where the patch cleanly applies can do the trick.
I think a heuristic would be to try py3k HEAD first. That's what most patches are supposed to work against.
Regards
Antoine.
On Sat, Oct 2, 2010 at 3:55 PM, "Martin v. Löwis" martin@v.loewis.dewrote:
I'll have to come up with a better way to determine the branch which a patch was created on.
That would also be helpful for those of us using DVCS software to talk to the svn server. :-)
Am 04.10.2010 03:56, schrieb Daniel Stutzbach:
On Sat, Oct 2, 2010 at 3:55 PM, "Martin v. Löwis" <martin@v.loewis.de mailto:martin@v.loewis.de> wrote:
I'll have to come up with a better way to determine the branch which a patch was created on.That would also be helpful for those of us using DVCS software to talk to the svn server. :-)
Not sure in what way that would be helpful: I know *have* a better way to determine the branch a patch was created on, but I completely fail to see how it would help the DVCS software users. I take the svn revision number from the patch, and then search back in history until I get a revision where all chunks patch cleanly without any changes to the line numbers; this has already helped a lot. I also have a database listing all file names, so I can deal with patches that were created for a subdirectory.
However, if I get something like
diff -r e981b6cc56b0 Include/longintrepr.h --- a/Include/longintrepr.h Thu Oct 07 03:12:19 2010 +0200 +++ b/Include/longintrepr.h Thu Oct 07 13:53:41 2010 +0200
I have no clue where I should look for the base revision that the patch was created against. I could guess that the base revision was probably created on Oct 07 3:12:19, which would make that r85299. Not sure how reliable this is, though - will the DVCS software always use the same time stamps in the diff for the base version as are recorded in the original repository?
Also, there are no directories called "a" and "b" in the repository. Will DVCS software always use these pseudo directories?
Regards, Martin
On Thu, 07 Oct 2010 23:04:54 +0200 "Martin v. Löwis" martin@v.loewis.de wrote:
However, if I get something like
diff -r e981b6cc56b0 Include/longintrepr.h --- a/Include/longintrepr.h Thu Oct 07 03:12:19 2010 +0200 +++ b/Include/longintrepr.h Thu Oct 07 13:53:41 2010 +0200
I have no clue where I should look for the base revision that the patch was created against.
As I said, most patches are supposed to be produced against py3k HEAD, so you could try just that as the primary heuristic.
I think you may trying too hard to find smart ways of inferring the correct svn rev and branch, while developing against latest py3k is, most of the time, the required standard. Outdated patches are not really helpful anyway.
Regards
Antoine.
As I said, most patches are supposed to be produced against py3k HEAD, so you could try just that as the primary heuristic.
I think this is impractical. There are tons of patches (the majority) which are in the tracker and *not* against py3k head. So this heuristics will only cover a small minority.
I think you may trying too hard to find smart ways of inferring the correct svn rev and branch, while developing against latest py3k is, most of the time, the required standard. Outdated patches are not really helpful anyway
Hmm. So how many versions should I go back in py3k until giving up?
Regards, Martin
Le jeudi 07 octobre 2010 à 23:17 +0200, "Martin v. Löwis" a écrit :
As I said, most patches are supposed to be produced against py3k HEAD, so you could try just that as the primary heuristic.
I think this is impractical. There are tons of patches (the majority) which are in the tracker and *not* against py3k head. So this heuristics will only cover a small minority.
I don't understand what the problem is. As long as the patch *applies* cleanly on py3k HEAD, it's ok. If it doesn't, the patch will have to be re-generated anyway.
I think you may trying too hard to find smart ways of inferring the correct svn rev and branch, while developing against latest py3k is, most of the time, the required standard. Outdated patches are not really helpful anyway
Hmm. So how many versions should I go back in py3k until giving up?
Zero.
Regards
Antoine.
Thanks Martin, this is really good.
On Sun, Oct 3, 2010 at 1:33 AM, "Martin v. Löwis" martin@v.loewis.de wrote:
Patches are skipped if: * they have been added to Rietveld already * they have no clear base version (i.e. they don't originate from svn diff) * they belong to no or a closed issue * they apply to a patch that is not currently supported (only trunk, 2.6, 2.7, 3.1, py3k are currently supported)
One comment:
I feel that, only if a roundup issue has patch, the corresponding rietveld issue be created, it is more helpful there and avoids needless duplication.
I followed the review link from issue5109 to arrive at http://bugs.python.org/review/5109/patch/179/325 . On the review page I clicked on Modules/arraymodule.c > View for side-by-side diff and got
Error fetching None/Modules/arraymodule.c?rev=83179: InvalidURLError: Protocol '%s' is not supported.
On the other hand, the unified diff link works.
Note that the "Tracker Branch" shows /branches/release27-maint even though the patch is for py3k.
On Tue, Oct 5, 2010 at 11:31 AM, "Martin v. Löwis" martin@v.loewis.de wrote:
I feel that, only if a roundup issue has patch, the corresponding rietveld issue be created, it is more helpful there and avoids needless duplication.
I have changed that now.
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/alexander.belopolsky%40gma...
Am 05.10.10 20:15, schrieb Alexander Belopolsky:
I followed the review link from issue5109 to arrive at http://bugs.python.org/review/5109/patch/179/325 . On the review page I clicked on Modules/arraymodule.c> View for side-by-side diff and got
Error fetching None/Modules/arraymodule.c?rev=83179: InvalidURLError: Protocol '%s' is not supported.
That currently happens for a lot of patches. It can't figure out what the base branch is, and goes to the Rietveld Issue object - on which it is None (because I believe Rietveld is misguided in associating base URLs with issues - they belong to patchsets, as different patches on the same issues might work on different branches).
On the other hand, the unified diff link works.
It already has the unified diff - just not the base text that it was generated from (and hence also not the new text).
Note that the "Tracker Branch" shows /branches/release27-maint even though the patch is for py3k.
Yes - it miscomputed it, based on the revision number in the patch (which was a revision in which 2.7 was modified).
Regards, Martin
On Tue, Oct 5, 2010 at 3:25 PM, "Martin v. Löwis" martin@v.loewis.de wrote: ..
Error fetching None/Modules/arraymodule.c?rev=83179: InvalidURLError: Protocol '%s' is not supported.
That currently happens for a lot of patches. It can't figure out what the base branch is, and goes to the Rietveld Issue object - on which it is None (because I believe Rietveld is misguided in associating base URLs with issues - they belong to patchsets, as different patches on the same issues might work on different branches).
I attempted to fix the branch at http://bugs.python.org/file18220 , but it did not trigger rietveld update. I assume you made "Tracker Branch" editable on purpose, but there should be a way rietveld side to know when this field is changed I think.
I attempted to fix the branch at http://bugs.python.org/file18220 , but it did not trigger rietveld update. I assume you made "Tracker Branch" editable on purpose, but there should be a way rietveld side to know when this field is changed I think.
Please start submitting issues with the Rietveld integration into the meta tracker.
Regards, Martin
participants (6)
-
"Martin v. Löwis" -
Alexander Belopolsky -
Antoine Pitrou -
Daniel Stutzbach -
Giampaolo Rodolà -
Senthil Kumaran