Summary

Corrected work with encoding on subversion and generate diff

Review Request #6205 — Created Aug. 7, 2014 and discarded Aug. 12, 2014, 2:57 a.m.

Information

Owner

1tush

Repository

Review Board

Branch

master

Bugs

Depends On

Commit

934db59...

Reviewers

Groups

reviewboard

People

Description

When repository encoding different from UTF-8, applying unicode (utf-8) diff to non utf-8 original file may cause an error (reviewboard/diffviewer/diffutils.py 'patch' function).
Also in code of both subversion client encoding of repository was hardcoded.

Testing Done

Issues

Description	From	Last Updated
list comprehension redefines 'file_attachment' from line 583	reviewbot	Aug. 7, 2014, 1:52 a.m.
undefined name '_'	reviewbot	Aug. 7, 2014, 1:52 a.m.
undefined name '_'	reviewbot	Aug. 7, 2014, 1:52 a.m.
Col: 80 E501 line too long (83 > 79 characters)	reviewbot	Aug. 7, 2014, 1:52 a.m.
Might as well use a list, since this is called encoding_list	david	Aug. 11, 2014, 12:24 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/webapi/resources/base_patched_file.py
    reviewboard/scmtools/svn/pysvn.py
    reviewboard/scmtools/svn/base.py
    reviewboard/webapi/resources/base_original_file.py
    reviewboard/diffviewer/chunk_generator.py
    reviewboard/diffviewer/diffutils.py
    reviewboard/scmtools/svn/subvertpy.py
    reviewboard/scmtools/svn/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/webapi/resources/base_patched_file.py
    reviewboard/scmtools/svn/pysvn.py
    reviewboard/scmtools/svn/base.py
    reviewboard/webapi/resources/base_original_file.py
    reviewboard/diffviewer/chunk_generator.py
    reviewboard/diffviewer/diffutils.py
    reviewboard/scmtools/svn/subvertpy.py
    reviewboard/scmtools/svn/__init__.py

reviewboard/reviews/views.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 list comprehension redefines 'file_attachment' from line 583
```
reviewboard/webapi/resources/base_original_file.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 undefined name '_'
```
reviewboard/webapi/resources/base_patched_file.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 undefined name '_'
```
reviewboard/webapi/resources/base_patched_file.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (83 > 79 characters)
```

Change Summary:

fix pep8 and pyflake issues

Commit:

934db59fea105f3fbd2a247fdb5f76c3f561f221

Diff:

Revision 2 (+26 -16)

Show changes

	reviewboard/diffviewer/chunk_generator.py
	reviewboard/diffviewer/diffutils.py
	reviewboard/reviews/views.py
	reviewboard/scmtools/svn/__init__.py
	reviewboard/scmtools/svn/base.py
	reviewboard/scmtools/svn/pysvn.py
	reviewboard/scmtools/svn/subvertpy.py
	reviewboard/webapi/resources/base_original_file.py
	1 more

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/webapi/resources/base_patched_file.py
    reviewboard/scmtools/svn/pysvn.py
    reviewboard/scmtools/svn/base.py
    reviewboard/webapi/resources/base_original_file.py
    reviewboard/diffviewer/chunk_generator.py
    reviewboard/diffviewer/diffutils.py
    reviewboard/scmtools/svn/subvertpy.py
    reviewboard/scmtools/svn/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/webapi/resources/base_patched_file.py
    reviewboard/scmtools/svn/pysvn.py
    reviewboard/scmtools/svn/base.py
    reviewboard/webapi/resources/base_original_file.py
    reviewboard/diffviewer/chunk_generator.py
    reviewboard/diffviewer/diffutils.py
    reviewboard/scmtools/svn/subvertpy.py
    reviewboard/scmtools/svn/__init__.py

Something about the way you updated the diff isn't correct. Revision 2 seems to use your first revision as a parent diff, instead of just containing a full diff between upstream and your code.

1tush Aug. 8, 2014, 4:47 a.m.

I've updated diff, but got an 500 error during publishing changes.

Change Summary:

Updated diff (now revision3 contains full list of changes)

Commit:

934db59fea105f3fbd2a247fdb5f76c3f561f221

Diff:

Revision 3 (+46 -28)

Show changes

	reviewboard/diffviewer/chunk_generator.py
	reviewboard/diffviewer/diffutils.py
	reviewboard/reviews/views.py
	reviewboard/scmtools/svn/__init__.py
	reviewboard/scmtools/svn/base.py
	reviewboard/scmtools/svn/pysvn.py
	reviewboard/scmtools/svn/subvertpy.py
	reviewboard/webapi/resources/base_original_file.py
	1 more

Fix it!

I haven't looked too closely at the code yet, because your "Testing done" section is not filled out. For a change like this, we really need to know what testing steps you've taken already.

reviewboard/scmtools/svn/base.py (Diff revision 3)
An issue was opened. Show all issues
```
Might as well use a list, since this is called encoding_list
```

Status:: Discarded