Review Request #34:

Summary

Review Request #34 — Created May 28, 2007 and discarded

Information

Owner

lobbin

Repository

Review Board SVN (deprecated)

Branch

trunk

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

A patch that adds an extra field file while parsing because the CVS tool I'm working on is needing this as lsdiff strips this field from the uploaded diff.

I'm not sure this is the "right" way to do it, but atleast I've done something that can be changed :-)

Testing Done

It works for me(tm). I've only tested on my own scmtool.

```
This is a good start.
```

trunk/reviewboard/diffviewer/parser.py (Diff revision 1)

Is there any way to do this inside the UploadDiffForm and avoid passing the tool to parse() here?

lobbin June 3, 2007, 6:17 a.m.

Ok, uploaded new patch which moves completly the parsing into the scmtool.

trunk/reviewboard/diffviewer/forms.py (Diff revision 1)

I'm not fond of this special casing.  Can we just change everything to add a third argument that gets ignored in the svn/perforce tools?

trunk/reviewboard/scmtools/core.py (Diff revision 1)
```
Trailing whitespace here.
```

On top of what David said, a couple small issues.

trunk/reviewboard/diffviewer/parser.py (Diff revision 1)
```
Should be 4 spaces here as well.
```
trunk/reviewboard/diffviewer/forms.py (Diff revision 1)
```
4 space indentation.
```

Is this the entire diff?  Even without your tool code (which I'd love to see), it seems like this needs something in the diffparser to populate the "content" field of the file.