fuzan/rietveld

Automatically exported from code.google.com/p/rietveld

rietveld's Issues

very large font, hard to read diffs

The font used for diffs as diplayed in IE is huge. The right hand file is 
not even visible in the window without scrolling. It is about 30% taller 
than the font Trac uses for its diffs. The inline-diff format that Trac 
uses would solve the horizontal scrolling issue as well. 

Original issue reported on code.google.com by [email protected] on 9 May 2008 at 2:47

Getting "error: old chunk mismatch" when trying to view issue 2822

What steps will reproduce the problem?
1. Go to http://codereview.appspot.com/2822
2. Click on any "Side-by-side diffs with inline comments"
3. See "error: old chunk mismatch" instead of visual diff

What is the expected output? What do you see instead?
visual diff expected, see "error: old chunk mismatch" instead


What version of the product are you using? On what operating system?
current - viewing from firefox 3.0.1 from OS X (10.5.4)

Please provide any additional information below.

Original issue reported on code.google.com by [email protected] on 5 Aug 2008 at 5:08

upload fails if there is no diff.exe in your path

upload runs svn diff like this:
  data = RunShell("svn diff --diff-cmd=diff", args)
It makes sense to override any GUI diffing program, but this also means 
that upload fails if you don't have a diff in your path.  However, "svn 
diff" works fine as it uses its own built-in differ.

(This is on Windows, where it's not common to have "diff" available.)

Perhaps there's some way to tell svn to use its internal diff 
implementation?  Or perhaps there's some way to test whether svn's default 
implementation has been overriden?

Original issue reported on code.google.com by [email protected] on 9 Jul 2008 at 9:32

Please allow - (dash) in change names

gmail will not index the individual words in an underscore-separated word
sequence.  this means that the individual words in the name of a change
cannot be searched for unless they also appear in the description.  It
would be nice if gcl allowed dashes in change names so they were more gmail
compatible.

Original issue reported on code.google.com by erik.corry on 17 Sep 2008 at 9:47

Issue 2412 contains a SVN diff that can't be applied

What steps will reproduce the problem?
1. Upload a patch with svn:keyword changes
2. Go to the last file in the patch before the svn:keyword change
3. Type j to go to the next page

What is the expected output? What do you see instead?
I expect to either see th svn:keyword change or the next file.
Instead I get an error: "Can't parse the patch"

Please provide any additional information below.

To see the error check:
http://codereview.appspot.com/2412/diff/1/8?context=50
Where typing j takes you to:
http://codereview.appspot.com/2412/diff/1/5?context=50
Which shows the error

Original issue reported on code.google.com by [email protected] on 25 Jun 2008 at 3:41

500 error returned by /upload containing non-ASCII characters

I made some changes to files in
https://googletransitdatafeed.googlecode.com/svn/trunk/python
In my first attempt the changes included a binary file (a zip) and
non-ascii in utf8. I tried to upload them by running

~/bin/codereview-upload.py   -m 'Put utf8 back in good_feed, sync zip, fix
google_random_queries' -e [email protected]
and got this stack dump:
Password for [email protected]:
Saving authentication cookies to /home/thecap/.codereview_upload_cookies
Traceback (most recent call last):
  File "/home/thecap/bin/codereview-upload.py", line 536, in ?
    main()
  File "/home/thecap/bin/codereview-upload.py", line 528, in main
    RealMain(sys.argv)
  File "/home/thecap/bin/codereview-upload.py", line 521, in RealMain
    response_body = rpc_server.Send("/upload", body, content_type=ctype)
  File "/home/thecap/bin/codereview-upload.py", line 270, in Send
    f = self.opener.open(req)
  File "/usr/lib/python2.4/urllib2.py", line 364, in open
    response = meth(req, response)
  File "/usr/lib/python2.4/urllib2.py", line 471, in http_response
    response = self.parent.error(
  File "/usr/lib/python2.4/urllib2.py", line 402, in error
    return self._call_chain(*args)
  File "/usr/lib/python2.4/urllib2.py", line 337, in _call_chain
    result = func(*args)
  File "/usr/lib/python2.4/urllib2.py", line 480, in http_error_default
    raise HTTPError(req.get_full_url(), code, msg, hdrs, fp)
urllib2.HTTPError: HTTP Error 500: Internal Server Error

I then reverted my zip file change and tried again. A similar stackdump was
returned:
Traceback (most recent call last):
  File "/home/thecap/bin/codereview-upload.py", line 536, in ?
    main()
  File "/home/thecap/bin/codereview-upload.py", line 528, in main
    RealMain(sys.argv)
  File "/home/thecap/bin/codereview-upload.py", line 521, in RealMain
    response_body = rpc_server.Send("/upload", body, content_type=ctype)
  File "/home/thecap/bin/codereview-upload.py", line 270, in Send
    f = self.opener.open(req)
  File "/usr/lib/python2.4/urllib2.py", line 364, in open
    response = meth(req, response)
  File "/usr/lib/python2.4/urllib2.py", line 471, in http_response
    response = self.parent.error(
  File "/usr/lib/python2.4/urllib2.py", line 402, in error
    return self._call_chain(*args)
  File "/usr/lib/python2.4/urllib2.py", line 337, in _call_chain
    result = func(*args)
  File "/usr/lib/python2.4/urllib2.py", line 480, in http_error_default
    raise HTTPError(req.get_full_url(), code, msg, hdrs, fp)
urllib2.HTTPError: HTTP Error 500: Internal Server Error

At http://codereview.appspot.com/ I see that each attempt created 4 issues.
768

test issue without binary delta

me


0 minutes
734

test issue without binary delta

me


0 minutes
886

test issue without binary delta

me


0 minutes
851

test issue without binary delta

me


0 minutes
850

Put utf8 back in good_feed, sync zip, fix google_random_queries

me


6 minutes
868

Put utf8 back in good_feed, sync zip, fix google_random_queries

me


6 minutes
711

Put utf8 back in good_feed, sync zip, fix google_random_queries

me


6 minutes
826

Put utf8 back in good_feed, sync zip, fix google_random_queries

me


6 minutes

Original issue reported on code.google.com by [email protected] on 6 May 2008 at 4:00

assertion error on comment reply

What steps will reproduce the problem?
1. Go to http://codereview.appspot.com/3055/diff/1/5?context=50
2. When I tried to reply to GvR comment (Reply button) with Konqueror (web
browser) I get an assertion error without any extra information !?

I retried with Firefox and it worked.

I'm using Ubuntu Gutsy with Firefox 2.0.0.16 and Konqueror 3.5.8.

Original issue reported on code.google.com by [email protected] on 23 Aug 2008 at 10:11

Doesn't handle files with CRLF newline endings

Rietveld doesn't seem to handle diffs of files with CRLF endings.  I had a
file with CRLF line endings and no svn:eol-style property in my svn
repository; I checked it out and edited it on my Mac, and then uploaded it
to Rietveld.  The diff had CRLFs in it because the original source file
did.  I was able to view the raw diffs in Rietveld, but when I clicked on
"0 comments" to see its diff view, I got an error message: "Bad content.
Try to upload again."

Original issue reported on code.google.com by [email protected] on 19 Aug 2008 at 12:30

'no recognizable patchsets'

A friend sent me a patch (against svn itself!), and rietveld's web ui tells
me it "has no recognizable patchsets in it" when I try to upload it.

Patch is attached.

Original issue reported on code.google.com by [email protected] on 1 Aug 2008 at 6:57

Attachments:

• pre-commit-lock-tokens.patch

SVN urls don't get the revision properly

What steps will reproduce the problem?
Go to http://codereview.appspot.com/5076/diff/1/6

What is the expected output? What do you see instead?
Expected diff. Instead see "error: old chunk mismatch"

Please provide any additional information below.
I believe that the SVN URLs aren't correct.

When using an invalid repository base, I saw how the URL was constructed,
and it didn't seem correct.

http://codereview.appspot.com/5078/diff/1/6
has an error in repository base, implies that repository is using URL
http://svn.apache.org/repos/asf/incubator/shindig/trunk/java/server/src/test/jav
a/org/apache/shindig/server/endtoend/EndToEndServer.java?rev=685757
for the old version.

This URL doesn't respect the version number (remove rev= and the content is
the same)

Original issue reported on code.google.com by [email protected] on 17 Sep 2008 at 8:02

Django fails to load on Windows with "AttributeError: 'module' object has no attribute 'unlink'"

What steps will reproduce the problem?

1. Install Python 2.5.2 on Windows (XP in my case)
2. Setup PATHs, etc for GAE
3. Run rietveld.py serve (or any other method of starting rietveld)
4. Navigate to http://localhost:<port>/


What is the expected output? What do you see instead?

I expect it to load the Issues list as per normal.
Instead, I get the exception as listed in the Issue summary


What version of the product are you using? On what operating system?

GAE 1.1.2, latest trunk of rietveld (revision 311)


Please provide any additional information below.

Basically it's due to Django's use of os.unlink in 
django/core/files/temp.py, line 34

When the django module is imported in rietveld/main.py, this line throws 
because os.unlink is not available in GAE.

This could be solved either by patching Django to check for the existence 
of os.unlink (arguably a bit overkill for such an obscure situation), or by 
making it a noop in rietveld.

Attached is a patch for the latter. This solves the issue for me.

Original issue reported on code.google.com by [email protected] on 16 Sep 2008 at 6:11

Attachments:

• make_unlink_noop.patch

access control: make review private or restrict to project.

This is just a feature request. While I understand that it's possible to
get a GAE account (or soon enough) and deploy one's own version of rietveld
that would be restricted to whoever you would want, I think that some
access controls should be built in.

One access control feature would be to allow people to keep code reviews
private and only visible to whomever is reviewing. This could be similar to
how other Google applications like Google Docs and Google Calendar work.

Another nice option would be to restrict code review viewing to project
members by integrating with Google Code. Anyone would be able to submit a
review but only project members will be able to review it.

This App looks very promising and we might start using it soon. Thank you
for the great work!

Arthur Kalmenson

Original issue reported on code.google.com by [email protected] on 7 May 2008 at 6:26

Remove "Download" link for each patch

Chromium's Rietveld's instance is using about 100 MB of storage a week, and 
I'm looking at reducing that.

One thing I'm looking at it is reusing base files when they're the same as 
earlier patchsets.  This doesn't affect Rietveld's UI.

One other idea though is to remove the "Download" beside each file.  I 
don't think it's needed, since if someone wants to apply a patch they will 
most probably want to apply it to all the files.  If someone really wants 
to do it to just one file, then they can edit the full patch themself.  Are 
people ok with this change?

Original issue reported on code.google.com by [email protected] on 21 Sep 2008 at 6:40

Some 'svn info' Repository Roots have a username@ in front of the host name

Some svn clients (a MacOS X GUI one for SVN 1.5, in this case) prepend a
username@ between the http[s]: protocol and the host name.  Rietveld
responds with this error when attempting to view the side-by-side diff:

Error fetching
http://[email protected]/svn-history/r406/trunk//app/settings.py:
InvalidURLError: ApplicationError: 1

Removing the username@ portion using the Google App Engine Data Viewer
makes the problem go away.

This patch fixes the problem for Google Code svn repos.  Not sure if it
really belongs elsewhere in GuessBase(), though.

Index: upload.py
===================================================================
--- upload.py   (revision 253)
+++ upload.py   (working copy)
@@ -609,6 +609,16 @@
           base = url + "/"
           if base.startswith("https"):
             base = "http" + base[5:]
+          # some 'svn info' Repository Roots contain a username@
+          username_match = re.match(
+              r'''(?x)
+                  (http://)  # group 1: protocol (keep)
+                  ([^/]+@)   # group 2: username@ (to be discarded)
+                  (.*)       # group 3: everything else (keep)
+               ''', base)
+          if username_match:
+            # omit group matching username@, removing it
+            base = username_match.group(1) + username_match.group(3)
           logging.info("Guessed Google Code base = %s", base)
         else:
           base = url + "/"

Original issue reported on code.google.com by todd.larsen on 18 Aug 2008 at 11:23

"Can't find URL in output from svn info"

upload.py fails when trying to parse the result from svn info. It succeeds
if I type "LANGUAGE=C upload.py". upload.py should rely on "svn info --xml"
instead.

For information, this is what svn info looks like here:

Chemin : .
URL : http://svn.python.org/projects/python/branches/py3k
Racine du dépôt : http://svn.python.org/projects
UUID du dépôt : 6015fed2-1504-0410-9fe1-9d1591cc4771
Révision : 64591
Type de nœud : répertoire
Tâche programmée : normale
Auteur de la dernière modification : bill.janssen
Révision de la dernière modification : 64588
Date de la dernière modification: 2008-06-29 02:08:12 +0200 (dim. 29 juin 2008)

Original issue reported on code.google.com by antoine.pitrou on 29 Jun 2008 at 8:47

Diff of empty added file causes 500 error

To reproduce: add an empty file to a project with svn add; then upload it
with upload.py. Then view the side-by-side diff for that file.

Original issue reported on code.google.com by [email protected] on 9 May 2008 at 3:12

upload.py should allow me to specify reviewers and have them autoemailed

It would be nice if when I ran upload.py it allowed me to specify my reviewers 
and have them 
emailed (and cc's) so that it made it more of a onestep process.

Original issue reported on code.google.com by dmaclach on 8 May 2008 at 9:38

No support for binary files

Any patch involving a binary file is likely to elicit 500 server errors.

Original issue reported on code.google.com by [email protected] on 7 May 2008 at 10:01

GETs should be idempotent

What steps will reproduce the problem?

1. As the issue owner, navigate to (ie, perform an HTTP GET for) 
http://<reitveld>/<issue_id>/delete

What is the expected output? What do you see instead?

I would expect to be denied and for no action to occur (eg, see a 405 error). 
Instead, the issue is 
deleted. This has consequences for any "eager fetching" caching applications.

Checks should be added to all non-idempotent methods to ensure that only POSTs, 
DELETEs, etc 
can modify data.

What version of the product are you using? On what operating system?

1.1.2, Mac OS X (only tested on development environment)

Please provide any additional information below.

"... the GET and HEAD methods SHOULD NOT have the significance of taking an 
action other than 
retrieval."
http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html

Original issue reported on code.google.com by [email protected] on 28 Aug 2008 at 12:49

RSS Feed for Recent Issues

What steps will reproduce the problem?
1. Open the Recent Issues page
2. Look for the autodiscovery RSS icon to subscribe

What is the expected output? What do you see instead?
A small rss button on my firefox :)

What version of the product are you using? On what operating system?
nothing

Please provide any additional information below.

It would be really useful to subscribe to some project's review queue to
keep track of what is going on. 

Original issue reported on code.google.com by fabricio on 15 Sep 2008 at 2:42

Server error 500 for files that are added (or added as part of rename)

What steps will reproduce the problem?
1. Have some file, lets say foo.c, in svn repository
2. Do svn rename foo.c bar.c (I think "svn add" will have same issues)
3. Use upload.py to upload the patch.
4. try to see the code of new file
5. You'll see 500 code page

What is the expected output? What do you see instead?
Content of newly-added file

What version of the product are you using? On what operating system?
Windows, current version on codereview.appspot.com

Please provide any additional information below.

Original issue reported on code.google.com by [email protected] on 7 Jun 2008 at 12:59

upload.py should allow specification of server port

1. start app engine development server (which starts up on port 8080)
2. run upload.py -s localhost

It fails with (partial trace):

"  File "/usr/local/lib/python2.5/urllib2.py", line 1075, in do_open
    raise URLError(err)
urllib2.URLError: <urlopen error (111, 'Connection refused')>"

The reason is that upload.py tries to connect to port 80 and it fails. I
tried the following and it does make upload.py use port 8080.

$ upload.py -s localhost:8080

If this is acceptable, we only need to change the option description. If
not, code may have to be tweaked to accept and use port number. I can work
on it in either case.

Original issue reported on code.google.com by [email protected] on 9 May 2008 at 7:50

Add a 'handled' / 'done' button for issues.

What steps will reproduce the problem?
1. Go to an issue
2. Handle it
3. Look for the 'done' button

What is the expected output? What do you see instead?
A done button that removes the issue from 'recent issues'.

What version of the product are you using? On what operating system?
The latest.

Original issue reported on code.google.com by [email protected] on 8 May 2008 at 11:01

Delta should not be shown for unchanged files

"delta from patch set X" is shown for all files after uploading a diff for
an exiting issue. There is no way to tell if that file has changed until
opening the patch. And file is shown completely in side by side format. Not
showing the delta for unchanged files could save some time for the reviewers.

Original issue reported on code.google.com by [email protected] on 28 May 2008 at 11:38

Add source highlighting

It would be nice if Rietveld used pygments or some such library to
highlight sources for review. Thanks!

Original issue reported on code.google.com by [email protected] on 10 Jun 2008 at 8:44

svn+ssh:// support

There should be support to add svn+ssh:// and ssh:// branches/repositories.

For a couple of projects I develop on we have http or https for anonymous
and svn+ssh for private. I'm using the upload.py tool which uses svn info
to fetch the url for the private branch while rietvald uses the public http
protocol to show the files anonymously.

Could it be related to urllib refusing to parse svn+ssh urls? You might
want to look at bzr-svn which extends the internal urllib list.

Original issue reported on code.google.com by [email protected] on 3 Jun 2008 at 8:24

text area expander doesn't work in Google Chrome

What steps will reproduce the problem?
1. Click on "Publish+Mail Comments" on any issue.
2. Click the blue plus next to the text area box
3. nothing happens

What is the expected output? What do you see instead?
The text area get bigger.

Alternately, on browsers that support resizeable text areas (webkit based 
browsers), we should just not show the blue plus because it's redundant.

Original issue reported on code.google.com by [email protected] on 23 Sep 2008 at 12:06

can't update sane entity twice in a transaction

#

1.
      05-07 08:37AM 41.948 /new 500 1153ms 0 kb ...
      See details

      218.167.69.56 - scw1109 [07/05/2008:08:37:43 -0700] "POST /new
HTTP/1.1" 500 26 "http://codereview.appspot.com/new" -

2.
      E 05-07 08:37AM 43.74 Exception in request: BadRequestError: can't updat

      Exception in request: BadRequestError: can't update the same entity
twice in a transaction
      Traceback (most recent call last):
        File
"/base/data/home/apps/codereview/1.157/django/core/handlers/base.py", line
82, in get_response
          response = callback(request, *callback_args, **callback_kwargs)
        File "/base/data/home/apps/codereview/1.157/views.py", line 251, in
login_wrapper
          return func(request, *args, **kwds)
        File "/base/data/home/apps/codereview/1.157/views.py", line 332, in new
          issue = _make_new(request, form)
        File "/base/data/home/apps/codereview/1.157/views.py", line 423, in
_make_new
          return db.run_in_transaction(txn)
        File
"/base/python_lib/versions/1/google/appengine/api/datastore.py", line 1319,
in RunInTransaction
          result = function(*args, **kwargs)
        File "/base/data/home/apps/codereview/1.157/views.py", line 414, in txn
          patchset.delete()
        File
"/base/python_lib/versions/1/google/appengine/ext/db/__init__.py", line
654, in delete
          datastore.Delete(self.key())
        File
"/base/python_lib/versions/1/google/appengine/api/datastore.py", line 255,
in Delete
          raise _ToDatastoreError(err)
        File
"/base/python_lib/versions/1/google/appengine/api/datastore.py", line 1499,
in _ToDatastoreError
          raise errors[err.application_error](err.error_detail)
      BadRequestError: can't update the same entity twice in a transaction

Original issue reported on code.google.com by [email protected] on 7 May 2008 at 3:55

Add a --reviewers option in upload.py

What steps will reproduce the problem?
1. Run upload.py
2. Go to the created issue
3. Look at the cc list

What is the expected output? What do you see instead?
That there is a specified cc present; instead there is no way to specify
the cc list through upload.py, this has to happen manually.

What version of the product are you using? On what operating system?
The latest version of upload.py.

Please provide any additional information below.
This would be really nice, especially in combination with 
http://code.google.com/p/rietveld/issues/detail?id=10 .

Original issue reported on code.google.com by [email protected] on 8 May 2008 at 9:38

Support for CC on emails

It would be nice to be able to add CCs for the review emails.  ie-set the
reviewer to someone to look at the code, but CC the google group for a
project so folks know about the coming code change, etc.

(might even make sense to allow configuring a repository(/branch) w/ a
default cc for all issues)

Original issue reported on code.google.com by [email protected] on 7 May 2008 at 8:58

Serious problem: everything is not working.

What steps will reproduce the problem?
1.
2.
3.

What is the expected output? What do you see instead?


What version of the product are you using? On what operating system?


Please provide any additional information below.

Original issue reported on code.google.com by [email protected] on 28 Jul 2008 at 11:56

upload.py -e only works with dev_appserver

I ran upload.py -e [email protected] and was still prompted for email.

Here is a patch:
--- upload.py.orig  2008-05-06 08:45:42.732344000 -0700
+++ upload.py       2008-05-06 08:47:17.407473000 -0700
@@ -373,7 +373,9 @@

   def GetUserCredentials():
     """Prompts the user for a username and password."""
-    email = raw_input("Email: ").strip()
+    email = options.email
+    if email is None:
+      email = raw_input("Email: ").strip()
     password = getpass.getpass("Password for %s: " % email)
     return (email, password)

Original issue reported on code.google.com by [email protected] on 6 May 2008 at 3:48

Uploading base file fails when it includes a delete of a binary file

What steps will reproduce the problem?
1. svn delete test.png
2. upload.py -l


What is the expected output?

Uploading test.png (1/1)


What do you see instead?

Uploading test.png (1/1)
  --> ERROR: Checksum mismatch2.f56c9ee8651b49115dae9ab59b0415a3.658e8dc0bf8b9a09b36994abf9242099.


The line that seem to be causing this is in views.py:
  checksum = md5.new(content.text.encode('utf-8')).hexdigest()

The problem is that line above
  content.text [+]= engine._ToText(data.splitlines(True))
tries to decode the data as utf-8, but if it has a decoding error, it uses 
latin-1.  For binary files, the 
latter seems to be hit (at least with the png that I tried).  Therefore the 
file is decoded with latin-1 
and saved to the database, then when it comes time to read it, it's encoded as 
utf-8 and so the checksum 
doesn't match.

This hack below gets around it, but I say it's a hack because it will fail if 
different parts of the file 
got decoded differently.  Not sure what the right solution is.  Andi, you know 
this code better than me, 
what do you think?

    checksum1 = md5.new(content.text.encode('utf-8')).hexdigest()
    checksum2 = md5.new(content.text.encode('latin-1')).hexdigest()
    if checksum1 != request.POST.get('checksum') and checksum2 != request.POST.get('checksum'):

Thanks

Original issue reported on code.google.com by [email protected] on 21 Jul 2008 at 8:19

Non-ASCII trouble when sending mail

Exception in request: UnicodeEncodeError: 'ascii' codec can't encode
character u'\xe8' in position 434: ordinal not in range(128)
Traceback (most recent call last):
  File
"/base/data/home/apps/codereview/1.160/django/core/handlers/base.py", line
82, in get_response
    response = callback(request, *callback_args, **callback_kwargs)
  File "/base/data/home/apps/codereview/1.160/views.py", line 274, in
issue_wrapper
    return func(request, *args, **kwds)
  File "/base/data/home/apps/codereview/1.160/views.py", line 251, in
login_wrapper
    return func(request, *args, **kwds)
  File "/base/data/home/apps/codereview/1.160/views.py", line 975, in publish
    reply_to=', '.join(everyone))
  File "/base/python_lib/versions/1/google/appengine/api/mail.py", line
205, in send_mail
    message.send(make_sync_call)
  File "/base/python_lib/versions/1/google/appengine/api/mail.py", line
452, in send
    make_sync_call('mail', self._API_CALL, message, response)
  File
"/base/python_lib/versions/1/google/appengine/api/apiproxy_stub_map.py",
line 47, in MakeSyncCall
    stub.MakeSyncCall(service, call, request, response)
  File "/base/python_lib/versions/1/google/appengine/runtime/apiproxy.py",
line 244, in MakeSyncCall
    rpc.MakeCall(package, call, request, response)
  File "/base/python_lib/versions/1/google/appengine/runtime/apiproxy.py",
line 144, in MakeCall
    self.request.Output(e)
  File "/base/python_lib/versions/1/google/net/proto/ProtocolBuffer.py",
line 147, in Output
    self.OutputUnchecked(e)
  File
"/base/python_lib/versions/1/google/appengine/api/mail_service_pb.py", line
814, in OutputUnchecked
    out.putPrefixedString(self.textbody_)
  File "/base/python_lib/versions/1/google/net/proto/ProtocolBuffer.py",
line 328, in putPrefixedString
    a.fromstring(v)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xe8' in
position 434: ordinal not in range(128)

Original issue reported on code.google.com by [email protected] on 7 May 2008 at 7:37

Add support for including/excluding files in upload.py

It would be great if you could specify which files in the generated diff should 
be part of the issue.

The gvn folks (http://code.google.com/p/gvn/) have a text editor pop up 
mid-operation to let you 
move file names above/below the "in" line do this.

Original issue reported on code.google.com by [email protected] on 9 May 2008 at 1:15

(Near) Pep8 Code

Being the stickler I am for common code style and knowing Google uses 2
spaces instead of 4 I've attached a patch that updates the style of the
code to be (even) closer to pep8 while still holding Google's 2 space
requirements.

The patch was created off of trunk r311 via svn diff >
r311-with-some-pep8-updates.patch

Original issue reported on code.google.com by [email protected] on 14 Sep 2008 at 9:46

Attachments:

• r311-with-some-pep8-updates.patch

some odd svn repositories

Looking at http://codereview.appspot.com/repos, I see a few googlecode.com
entries that end in "svn/trunk/trunk/".  This seems wrong?

Original issue reported on code.google.com by [email protected] on 7 May 2008 at 8:56

tesing

What steps will reproduce the problem?
1.
2.
3.

What is the expected output? What do you see instead?


What version of the product are you using? On what operating system?


Please provide any additional information below.

Original issue reported on code.google.com by [email protected] on 14 May 2008 at 12:09

Attachments:

• upload.py

allow to specify svn revision instead of uploading patch

What steps will reproduce the problem?
1. In many cases, patches are not being used and code reviews may need to
happen on the actual code in svn

What is the expected output? What do you see instead?
It would be neat to be able to specify an svn revision and let tool to
retrieve the diff

Original issue reported on code.google.com by [email protected] on 1 Aug 2008 at 8:03

Accept non-SVN diffs

One might work against an SVN repository with an non-SVN tool (e.g.
Mercurial, Bazaar). While a diff produced by hg or bzr won't mention the
SVN revision number, it is reasonable to assume that in most cases the
patch was produced against SVN HEAD (or a revision very similar to HEAD).
The patch could simply be rejected if applying it on SVN HEAD failed.

Original issue reported on code.google.com by antoine.pitrou on 14 Jun 2008 at 10:07

Django's FILE_UPLOAD_HANDLERS setting should only include MemoryFileUploadHandler

The default FILE_UPLOAD_HANDLERS settings include 
TemporaryFileUploadHandler, which is not usable on GAE

The attached patch rectifies this by explicitly setting FILE_UPLOAD_HANDLERS to 
only include MemoryFileUploadHandler.

This doesn't fully fix issue 44 (without a patch to Django), but it's a 
step in the right direction

Original issue reported on code.google.com by [email protected] on 17 Sep 2008 at 2:56

Attachments:

• explicit_FILE_UPLOAD_HANDLERS.patch

Cannot add comments using Opera.

What steps will reproduce the problem?
1. Using Opera 9.2x or 9.5, in side-by-side view, with opened comment form 
by double clicking a code line.
2. write some comment, then click "save" button,
3. it shows white screen only with "Error: AssertionError; please report!" 
in red. (using 9.5, side-by-side view with the error sendence.)

What is the expected output? What do you see instead?
commented side-by-side view.

What version of the product are you using? On what operating system?
1 minute ago.

Please provide any additional information below.
I wrote patch. plz check in.
http://codereview.appspot.com/2163

Original issue reported on code.google.com by [email protected] on 14 Jun 2008 at 6:18

script.js is broken because of a pending revisions conflict

What steps will reproduce the problem?
1. try a javascript related action on diff view (double-click to insert
comments).


What is the expected output? What do you see instead?
A comment insert form should appear, but nothing happens. The javascript
console complains:

illegal XML character
[Break on this error] <<<<<<< .mine\n
script.js (línea 2651)
M_clickCommon is not defined
[Break on this error] var intraLineDiff = new M_IntraLineDiff();
171 (línea 1734)
M_IntraLineDiff is not defined
[Break on this error] var intraLineDiff = new M_IntraLineDiff();
171 (línea 1734)
M_handleTableDblClick is not defined
[Break on this error] M_handleTableDblClick(event);


What version of the product are you using? On what operating system?
Current Google hosted version. Firefox on Linux on the client side.


Please provide any additional information below.
I add a patch to solve the conflict.

Original issue reported on code.google.com by [email protected] on 1 Sep 2008 at 12:01

If upload.py fails, it can leave behind an issue with an empty patch set

This is easily reproduced by attempting to upload a diff containing a non-ASCII 
character.

Original issue reported on code.google.com by [email protected] on 6 May 2008 at 6:15

500 Server Error and review email not sent

What steps will reproduce the problem?
1. Use upload.py to upload an svn diff.
2. Select the issue from Issues Created by Me
3. Select Publish+Mail Comments
4. Fill out the form, including a message.
5. Press Publish All My Drafts

What is the expected output? What do you see instead?

I expected an email to be sent with my message and a code review
invitation.  Instead, when I attempt to Publish + Mail Draft comments, I
get a 500 server error when I press Publish All My Drafts.

What version of the product are you using? On what operating system?

This is r52 of Rietveld from the svn repo.  It is running on a separate App
Engine instance at soc-codereviews.appspot.com.

Please provide any additional information below.

Here is the exception from the logs:

05-06 05:04PM 17.729 Exception in request: InvalidSenderError: Traceba

Exception in request: InvalidSenderError: 
Traceback (most recent call last):
  File
"/base/data/home/apps/soc-codereviews/1.1/django/core/handlers/base.py",
line 82, in get_response
    response = callback(request, *callback_args, **callback_kwargs)
  File "/base/data/home/apps/soc-codereviews/1.1/views.py", line 274, in
issue_wrapper
    return func(request, *args, **kwds)
  File "/base/data/home/apps/soc-codereviews/1.1/views.py", line 251, in
login_wrapper
    return func(request, *args, **kwds)
  File "/base/data/home/apps/soc-codereviews/1.1/views.py", line 952, in
publish
    reply_to=', '.join(everyone))
  File "/base/python_lib/versions/1/google/appengine/api/mail.py", line
205, in send_mail
    message.send(make_sync_call)
  File "/base/python_lib/versions/1/google/appengine/api/mail.py", line
455, in send
    raise ERROR_MAP[e.application_error]()
InvalidSenderError

Original issue reported on code.google.com by todd.larsen on 7 May 2008 at 12:31

upload.py repeatedly requests (and always fails) auth for a Google Apps-hosted instance of Rietveld

What steps will reproduce the problem?
1. run an instance of Rietveld on a Google Apps domain
2. run upload.py to upload a patch and open a new issue
3. can never authenticate (see below)

What is the expected output? What do you see instead?

The expected output is that upload.py would accept my valid email address
in the Google Apps domain that is hosting the instance of Rietveld
(googleopensourceprograms.com in this case), as it has for many weeks with
prior versions of Rietveld.  Instead, the only response is:

  Invalid username or password.

Both the email address ([email protected]) and the 
password have been verified repeatedly as correct.  Giving just a user
name (which I tried in the hopes that upload.py was already assuming the 
domain name somehow...) results instead in this response:

  Please go to
  https://www.google.com/accounts/DisplayUnlockCaptcha
  and verify you are a human.  Then try again.

I do not believe this CAPTCHA link is going to work for a Google Apps
hosted instance of Rietveld.

Original issue reported on code.google.com by todd.larsen on 12 Sep 2008 at 2:07

see the comments in the unified diff mode

Add the ability to see (maybe also add) comments in the unified diff mode.

More discussion can be found here:
 http://groups.google.com/group/codereview-discuss/browse_thread/thread/7bebb598690a8252

Original issue reported on code.google.com by [email protected] on 14 May 2008 at 8:23

SVN's getBaseFile fails for property changes with error "UnboundLocalError: local variable 'is_binary' referenced before assignment"

What steps will reproduce the problem?
1. change a property on a file or directory in an SVN repository
2. Use upload.py to create an issue with the changes
3. Upload of base file (now default option) will fail with error

What is the expected output? What do you see instead?
Expected output is complete upload of all base files

What version of the product are you using? On what operating system?
r318

Please provide any additional information below.
Traceback:

Traceback (most recent call last):
  File "../upload.py", line 1056, in <module>
    main()
  File "../upload.py", line 1048, in main
    RealMain(sys.argv)
  File "../upload.py", line 1040, in RealMain
    vcs.UploadBaseFiles(issue, rpc_server, patches, patchset, options)
  File "../upload.py", line 603, in UploadBaseFiles
    base_content, new_content, is_binary, status = self.GetBaseFile(filename)
  File "../upload.py", line 790, in GetBaseFile
    return base_content, new_content, is_binary, status[0:5]
UnboundLocalError: local variable 'is_binary' referenced before assignment


the reason seems to be that is_binary is never assigned a value during the 
property value change 
section of the if / elseif structure

{{{
    elif status[0] == " " and status[1] == "M":
      # Property changed, don't need to do anything.
      pass
}}}

By putting is_binary = False   at the top of the getBaseFile function i was 
able to avoid this 
problem

Original issue reported on code.google.com by [email protected] on 19 Sep 2008 at 3:29

http://code.google.com/appengine/images/appengine-transparent-120x30.gif not found

http://code.google.com/p/
rietveld/

shows an [image] beneath the 
signature, because the tracker 
image is not found and does not 
have an alt=""

What is the expected 
output? What do you see 
instead?

an image, or at least nothing 
at all.


What version of the product 
are you using? On what 
operating system?

Whatever was served just now -
- to Opera 9.27 for windows.

Please provide any 
additional information 
below.

Original issue reported on code.google.com by [email protected] on 4 May 2008 at 9:14

Add a 'All issues' view.

What steps will reproduce the problem?
1. Go to the main Rietveld page
2. Look for the 'All issues' field

What is the expected output? What do you see instead?
An 'All issues' view that shows all issues, including 'handled' ones (see
http://code.google.com/p/rietveld/issues/detail?id=14), preferably searchable. 

Original issue reported on code.google.com by [email protected] on 8 May 2008 at 11:04