rietveld - Code Review, hosted on Google App Engine

Last 7 days Feb 09, 2012 issue 322 (FIles with an ampersand in the name fail.) commented on by jpgorr...@gmail.com - That's not an ampersand... I think you mean 'ampersat' <sniff> That's not an ampersand... I think you mean 'ampersat' <sniff> Feb 09, 2012 issue 356 (Added binary image causes 500 error) commented on by albrecht.andi - Thanks for the feedback! Let us know when you've a patch ready for review! Thanks for the feedback! Let us know when you've a patch ready for review! Feb 09, 2012 issue 367 (comments that mention my name should be highlighted) Labels changed by techtonik@gmail.com - Everybody who quotes you in direct and non-direct followups will include your name in the reply. Maybe you want @name to be highlighted? Labels: Component-UI Everybody who quotes you in direct and non-direct followups will include your name in the reply. Maybe you want @name to be highlighted? Labels: Component-UI Feb 09, 2012 issue 366 (UploadPyUsage page needs example) Labels changed by techtonik@gmail.com - Labels: Component-Docs Easy Labels: Component-Docs Easy Feb 09, 2012 issue 28 (Issue 2412 contains a SVN diff that can't be applied) Labels changed by techtonik@gmail.com - Labels: Component-Engine Labels: Component-Engine Feb 08, 2012 issue 367 (comments that mention my name should be highlighted) reported by est...@chromium.org - feature request: if a comment mentions my username, that comment should be highlighted (similar to green highlighting for LGTM or yellow for #c in url) feature request: if a comment mentions my username, that comment should be highlighted (similar to green highlighting for LGTM or yellow for #c in url) Feb 08, 2012 issue 285 (git mv files don't list the deleted file, only the new one) commented on by sha...@google.com - The same is true for Mercurial. If I rename a file using "hg mv" I only see the new files in the Reitveld patch set description, and not the deleted files. The same is true for Mercurial. If I rename a file using "hg mv" I only see the new files in the Reitveld patch set description, and not the deleted files. Feb 08, 2012 issue 356 (Added binary image causes 500 error) commented on by sportjun...@gmail.com - No I have not signed the agreement. Have to do that at some point. Also I have not uploaded the patch either, it still has flaws with binary files. The patch only solves problems when adding a new binary image but the upload still fails for us when we try to upload a modified binary. We hacked an ugly try except into the upload.py to make it work but don't see it as a final solution. No I have not signed the agreement. Have to do that at some point. Also I have not uploaded the patch either, it still has flaws with binary files. The patch only solves problems when adding a new binary image but the upload still fails for us when we try to upload a modified binary. We hacked an ugly try except into the upload.py to make it work but don't see it as a final solution. Feb 07, 2012 issue 366 (UploadPyUsage page needs example) reported by techtonik@gmail.com - UploadPyUsage page badly needs a tutorial. Right now it contains only an autogenerated list of options. tag:easy UploadPyUsage page badly needs a tutorial. Right now it contains only an autogenerated list of options. tag:easy Feb 07, 2012 issue 142 (let reviewers close issues) commented on by rich.hol...@morphormics.com - Thanks for the comment by rsc, since it's better than nothing. And in case it helps, 'reply' here seems to mean clicking on the "Publish and mail comments" link for the case, after which you can either move names to CC OR delete them entirely, which means you won't get dead issues in your "Issues CCed to me" list in the "My issues" view. Still would like a real way to close issues for long-gone users... Thanks for the comment by rsc, since it's better than nothing. And in case it helps, 'reply' here seems to mean clicking on the "Publish and mail comments" link for the case, after which you can either move names to CC OR delete them entirely, which means you won't get dead issues in your "Issues CCed to me" list in the "My issues" view. Still would like a real way to close issues for long-gone users... Feb 06, 2012 2 new revisions pushed by maruel@chromium.org - 55a37729ee:Log more information on status_listener. Remove support for buildbot 0.7.12. Review: http://codereview.appspot.com/5616044/ fae51921ad:Merge default @ f802d7c33b51 55a37729ee:Log more information on status_listener. Remove support for buildbot 0.7.12. Review: http://codereview.appspot.com/5616044/ fae51921ad:Merge default @ f802d7c33b51 Feb 06, 2012 Revision f802d7c33b (Make sure color is not used in git diff or hg diff Review: ...) pushed by maruel@chromium.org - Make sure color is not used in git diff or hg diff Review: http://codereview.appspot.com/5619043/ Make sure color is not used in git diff or hg diff Review: http://codereview.appspot.com/5619043/ Feb 06, 2012 Revision 9ab36a1442 (Enforce content-disposition=attachment on image urls Review...) pushed by maruel@chromium.org - Enforce content-disposition=attachment on image urls Review: http://codereview.appspot.com/5607057/ Enforce content-disposition=attachment on image urls Review: http://codereview.appspot.com/5607057/ Feb 06, 2012 Revision 3a01a0c271 (Fix pylint errors introduced by merging branch default Revi...) pushed by maruel@chromium.org - Fix pylint errors introduced by merging branch default Review: http://codereview.appspot.com/5616076 Fix pylint errors introduced by merging branch default Review: http://codereview.appspot.com/5616076 Feb 06, 2012 Revision 422828ca47 (Merge default @ da188fb1e43c) pushed by maruel@chromium.org - Merge default @ da188fb1e43c Merge default @ da188fb1e43c Feb 06, 2012 Revision da188fb1e4 (Fix support for python 2.5 Review: http://codereview.appspo...) pushed by maruel@chromium.org - Fix support for python 2.5 Review: http://codereview.appspot.com/5630067/ Fix support for python 2.5 Review: http://codereview.appspot.com/5630067/ Feb 06, 2012 Revision 7d5d70c227 (Merge default @ a83b34c14322) pushed by maruel@chromium.org - Merge default @ a83b34c14322 Merge default @ a83b34c14322 Feb 06, 2012 Revision a83b34c143 (Enforce utf-8 on all text and html content Review: http://c...) pushed by maruel@chromium.org - Enforce utf-8 on all text and html content Review: http://codereview.appspot.com/5616046/ Enforce utf-8 on all text and html content Review: http://codereview.appspot.com/5616046/ Last 30 days Feb 06, 2012 Revision 3ff09a245d (Remove eternal TODO about getting rid of direct Django impor...) pushed by techtonik@gmail.com - Remove eternal TODO about getting rid of direct Django imports as it will require using full names for Django API and will make code less readable. As agreed on http://codereview.appspot.com/5552045/#msg11 Remove eternal TODO about getting rid of direct Django imports as it will require using full names for Django API and will make code less readable. As agreed on http://codereview.appspot.com/5552045/#msg11 Feb 03, 2012 issue 262 (Support patch upload from mercurial queue) commented on by gvanros...@gmail.com - Can't you use --rev rev1:rev2 ? Or is the problem that you have no way to figure out what rev1 and rev2 are? Can't you use --rev rev1:rev2 ? Or is the problem that you have no way to figure out what rev1 and rev2 are? Feb 03, 2012 issue 262 (Support patch upload from mercurial queue) commented on by gpsm...@gmail.com - What is the incantation sequence of commands to use upload.py when you've got your patch ready via "hg qdiff" today? I'd love for upload.py to support at the very least that simple common case. What is the incantation sequence of commands to use upload.py when you've got your patch ready via "hg qdiff" today? I'd love for upload.py to support at the very least that simple common case. Feb 03, 2012 CodeReviewHelp (Help for Rietveld, the code review web app at http://coderev...) Wiki page commented on by csegenyz...@gmail.com - Thank You guys! [http://www.bedbugbitesusa.com bed bugs] Thank You guys! [http://www.bedbugbitesusa.com bed bugs] Feb 03, 2012 CodeReviewHelp (Help for Rietveld, the code review web app at http://coderev...) Wiki page commented on by csegenyz...@gmail.com - Thank you guys! http://www.bedbugbitesusa.com Thank you guys! http://www.bedbugbitesusa.com Feb 01, 2012 issue 365 (Test bug report) Status changed by albrecht.andi - Works. Status: Invalid Works. Status: Invalid Feb 01, 2012 issue 365 (Test bug report) reported by shakalan...@gmail.com - What steps will reproduce the problem? 1. 2. 3. What is the expected output? What do you see instead? What browser are you using? What version? On what operating system? At what URL are you accessing Rietveld? (e.g. codereview.appspot.com) Please note if you are using the Google Apps Labs version (e.g. codereview.<yourdomain>). * If you are a Google employee please say so or mail me (Guido van Rossum) directly. * Please provide any additional information below. What steps will reproduce the problem? 1. 2. 3. What is the expected output? What do you see instead? What browser are you using? What version? On what operating system? At what URL are you accessing Rietveld? (e.g. codereview.appspot.com) Please note if you are using the Google Apps Labs version (e.g. codereview.<yourdomain>). * If you are a Google employee please say so or mail me (Guido van Rossum) directly. * Please provide any additional information below. Feb 01, 2012 issue 364 (filtering on open issues not maintained in pagination) reported by christin...@hiivesystems.com - What steps will reproduce the problem? 1.have more than 10 open issues 2.select to see open issues 3.click on the Older link What is the expected output? What do you see instead? expected to see the second tab of opened issues saw second tab of all issues when selecting to see open issues - it adds closed=0, to the link. Older adds closed=FALSE to the link - changing that to closed=0 produced what I wanted to see. What browser are you using? What version? On what operating system? chrome 16.0.912.77 on windows At what URL are you accessing Rietveld? (e.g. codereview.appspot.com) Please note if you are using the Google Apps Labs version (e.g. codereview.<yourdomain>). http://cr.hiivesystems.com * If you are a Google employee please say so or mail me (Guido van Rossum) directly. * Please provide any additional information below. What steps will reproduce the problem? 1.have more than 10 open issues 2.select to see open issues 3.click on the Older link What is the expected output? What do you see instead? expected to see the second tab of opened issues saw second tab of all issues when selecting to see open issues - it adds closed=0, to the link. Older adds closed=FALSE to the link - changing that to closed=0 produced what I wanted to see. What browser are you using? What version? On what operating system? chrome 16.0.912.77 on windows At what URL are you accessing Rietveld? (e.g. codereview.appspot.com) Please note if you are using the Google Apps Labs version (e.g. codereview.<yourdomain>). http://cr.hiivesystems.com * If you are a Google employee please say so or mail me (Guido van Rossum) directly. * Please provide any additional information below. Feb 01, 2012 Revision 65a6c2fe1a (Clean up a few unnecessary things.) pushed by guido@google.com - Clean up a few unnecessary things. Clean up a few unnecessary things. Feb 01, 2012 Revision 50a838b594 (Merge default @ a51a5831bfe9) pushed by maruel@chromium.org - Merge default @ a51a5831bfe9 Merge default @ a51a5831bfe9 Feb 01, 2012 Revision a51a5831bf (Enforce image/ content type to be served on 'image' url.) pushed by maruel@chromium.org - Enforce image/ content type to be served on 'image' url. Enforce image/ content type to be served on 'image' url. Jan 31, 2012 Revision f032dde933 (Merge) pushed by guido@google.com - Merge Merge Jan 31, 2012 3 new revisions pushed by guido@google.com - 4e876467a7:Early work towards Python 2.7 migration. 433eaea5e4:Replace djangoforms with equivalent manual code. 0628b9ceec:Merge default. 4e876467a7:Early work towards Python 2.7 migration. 433eaea5e4:Replace djangoforms with equivalent manual code. 0628b9ceec:Merge default. Jan 31, 2012 2 new revisions pushed by roge...@chromium.org - 184e9bf154:Fix problem with my commit that broke the reply link in messages 84cc73cbe2:Merging with MA's merge from default 184e9bf154:Fix problem with my commit that broke the reply link in messages 84cc73cbe2:Merging with MA's merge from default Jan 31, 2012 issue 348 (Python 2.7 support) commented on by gvanros...@gmail.com - Looks like they're about the same speed actually, so never mind about the /repos page. Looks like they're about the same speed actually, so never mind about the /repos page. Jan 31, 2012 Revision b6085b5112 (Merge branch default @ 96fda151c7b5) pushed by maruel@chromium.org - Merge branch default @ 96fda151c7b5 Merge branch default @ 96fda151c7b5 Jan 31, 2012 Revision 96fda151c7 (If no title is provided, use ' ' by default instead of error...) pushed by maruel@chromium.org - If no title is provided, use ' ' by default instead of erroring out Review: http://codereview.appspot.com/5577063/ If no title is provided, use ' ' by default instead of erroring out Review: http://codereview.appspot.com/5577063/ Jan 31, 2012 Revision 38a28c2729 (Fix regression with always showing 'Trybot results') pushed by roge...@chromium.org - Fix regression with always showing 'Trybot results' Fix regression with always showing 'Trybot results' Jan 31, 2012 issue 348 (Python 2.7 support) commented on by techtonik@gmail.com - Opening /repos page takes 6s on Python 2.5 I can create a separate issue if Python 2.7 will take longer. Opening /repos page takes 6s on Python 2.5 I can create a separate issue if Python 2.7 will take longer. Jan 30, 2012 issue 348 (Python 2.7 support) commented on by gvanros...@gmail.com - FWIW I've done the most basic port imaginable and it seems to work, although the /repos page is slow and I haven't turned on multi-threading. See these two code reviews: http://codereview.appspot.com/5574079/ - basic changes to app.yaml etc. http://codereview.appspot.com/5552045/ - deal with lack of djangoforms FWIW I've done the most basic port imaginable and it seems to work, although the /repos page is slow and I haven't turned on multi-threading. See these two code reviews: http://codereview.appspot.com/5574079/ - basic changes to app.yaml etc. http://codereview.appspot.com/5552045/ - deal with lack of djangoforms Jan 30, 2012 Revision 28f6645ed3 (Add entry to dos.yaml.) pushed by maruel@chromium.org - Add entry to dos.yaml. Add entry to dos.yaml. Jan 29, 2012 Revision 33bb90856e (Stop using djangoforms.) pushed by guido@google.com - Stop using djangoforms. Stop using djangoforms. Jan 26, 2012 issue 363 (Rietveld Code Review Tool bugs) commented on by gvanros...@gmail.com - I coded (2) intentionally this was. The browser has several features to force a download -- you can right-click or control-click and use "save link as", and you can save the displayed text easily via another menu items. I like to be able to preview what gets downloaded. If this is really too much for you to handle, maybe we should just change the label to "Show raw patch". I coded (2) intentionally this was. The browser has several features to force a download -- you can right-click or control-click and use "save link as", and you can save the displayed text easily via another menu items. I like to be able to preview what gets downloaded. If this is really too much for you to handle, maybe we should just change the label to "Show raw patch". Jan 26, 2012 issue 363 (Rietveld Code Review Tool bugs) commented on by techtonik@gmail.com - Niiice. Thanks for reporting. We now just need to find some time to confirm and _maybe_ split these into separate issues. I can confirm p.2, but could you in the meanwhile confirm that p.1 and p.3 are actual for http://codereview.appspot.com/ just in case you're using some other version (for example, http://bugs.python.org/review/ is different). And specify Rietveld revision used if that is the case (usually available in lower right corner). Niiice. Thanks for reporting. We now just need to find some time to confirm and _maybe_ split these into separate issues. I can confirm p.2, but could you in the meanwhile confirm that p.1 and p.3 are actual for http://codereview.appspot.com/ just in case you're using some other version (for example, http://bugs.python.org/review/ is different). And specify Rietveld revision used if that is the case (usually available in lower right corner). Jan 26, 2012 issue 363 (Rietveld Code Review Tool bugs) reported by jadamsbo...@gmail.com - The Rietveheld Code Review Tool has 3 bugs: 1) Rietveld Code Review Tool isn't able to delete a patch set. For the patches that I create it displays a "Delete patch set" option, but it is broken. Always displays the 500 error page. I am not able to delete any of my patches that I want. 2) The option "Download raw patch set" is not broken, however, but has a sementic error. It does not download anything, it just displays the patch in the browser, not offers to save a .diff or .patch file. Also, the individual file's "Download" option does not do the right thing as well. 3) When the "Delete patch set" option is present besides the "Download raw patch set", the former is squeezed to the delimiting symbol (the \| symbol). It looks like this: "Download raw patch set \|Delete patch set" which should have been displayed as "Download raw patch set \| Delete patch set" The Rietveheld Code Review Tool has 3 bugs: 1) Rietveld Code Review Tool isn't able to delete a patch set. For the patches that I create it displays a "Delete patch set" option, but it is broken. Always displays the 500 error page. I am not able to delete any of my patches that I want. 2) The option "Download raw patch set" is not broken, however, but has a sementic error. It does not download anything, it just displays the patch in the browser, not offers to save a .diff or .patch file. Also, the individual file's "Download" option does not do the right thing as well. 3) When the "Delete patch set" option is present besides the "Download raw patch set", the former is squeezed to the delimiting symbol (the \| symbol). It looks like this: "Download raw patch set \|Delete patch set" which should have been displayed as "Download raw patch set \| Delete patch set" Jan 24, 2012 issue 361 (Dataloss with "Invalid XSRF token" error) commented on by pkasting@chromium.org - Re: comment 6: I finally got around to retesting this. I pulled up the link from comment 5, left it open in a tab well over a day, hit the "reply" button and deleted all the content, then hit "send message". On getting the "invalid token" page I hit back, and hit "reply" again. The boilerplate text was all back rather than still being deleted. Re: comment 6: I finally got around to retesting this. I pulled up the link from comment 5, left it open in a tab well over a day, hit the "reply" button and deleted all the content, then hit "send message". On getting the "invalid token" page I hit back, and hit "reply" again. The boilerplate text was all back rather than still being deleted. Jan 24, 2012 Revision 58887ddf54 (Temporarily disable UI for trying a patchset on new bots.) pushed by roge...@chromium.org - Temporarily disable UI for trying a patchset on new bots. Temporarily disable UI for trying a patchset on new bots. Jan 24, 2012 Revision c864efbf45 (Add UI to allow sending try job to the try servers directly ...) pushed by roge...@chromium.org - Add UI to allow sending try job to the try servers directly from rietveld. This is UI only, the backend is not functioning yet. Also clean up the UI when multiple try jobs for the same builder are sent with the same patchset. Allow the user to see more/less. Add UI to allow sending try job to the try servers directly from rietveld. This is UI only, the backend is not functioning yet. Also clean up the UI when multiple try jobs for the same builder are sent with the same patchset. Allow the user to see more/less. Jan 23, 2012 Revision 827fa087f7 (Merge default @ 9349cab9a3bb) pushed by maruel@chromium.org - Merge default @ 9349cab9a3bb Merge default @ 9349cab9a3bb Jan 23, 2012 Revision 9349cab9a3 (Default --title on --message and clamp at 100 chars Review:...) pushed by maruel@chromium.org - Default --title on --message and clamp at 100 chars Review: http://codereview.appspot.com/5574050/ Default --title on --message and clamp at 100 chars Review: http://codereview.appspot.com/5574050/ Jan 22, 2012 Revision dbd68677ba (Don't handle Http404 as real exceptions.) pushed by albrecht.andi - Don't handle Http404 as real exceptions. Don't handle Http404 as real exceptions. Jan 21, 2012 Revision 4a2a205ac1 (Merge default @ 6bcce636d5a8.) pushed by maruel@chromium.org - Merge default @ 6bcce636d5a8. Merge default @ 6bcce636d5a8.

issue 363 (Rietveld Code Review Tool bugs) reported by jadamsbo...@gmail.com - The Rietveheld Code Review Tool has 3 bugs: 1) Rietveld Code Review Tool isn't able to delete a patch set. For the patches that I create it displays a "Delete patch set" option, but it is broken. Always displays the 500 error page. I am not able to delete *any* of my patches that I want. 2) The option "Download raw patch set" is not broken, however, but has a sementic error. It does not download anything, it just *displays* the patch in the browser, not offers to save a *.diff or *.patch file. Also, the individual file's "Download" option does not do the right thing as well. 3) When the "Delete patch set" option is present besides the "Download raw patch set", the former is squeezed to the delimiting symbol (the | symbol). It looks like this: "Download raw patch set |Delete patch set" which should have been displayed as "Download raw patch set | Delete patch set"

The Rietveheld Code Review Tool has 3 bugs: 1) Rietveld Code Review Tool isn't able to delete a patch set. For the patches that I create it displays a "Delete patch set" option, but it is broken. Always displays the 500 error page. I am not able to delete *any* of my patches that I want. 2) The option "Download raw patch set" is not broken, however, but has a sementic error. It does not download anything, it just *displays* the patch in the browser, not offers to save a *.diff or *.patch file. Also, the individual file's "Download" option does not do the right thing as well. 3) When the "Delete patch set" option is present besides the "Download raw patch set", the former is squeezed to the delimiting symbol (the | symbol). It looks like this: "Download raw patch set |Delete patch set" which should have been displayed as "Download raw patch set | Delete patch set"

Last 7 days

Last 30 days