Summary

Add support for Team Foundation Server repositories.

Review Request #6646 — Created Nov. 25, 2014 and submitted Dec. 22, 2014, 2:06 p.m.

Information

Owner

david

Repository

RBTools

Branch

master

Bugs

Depends On

Commit

a34f92e...

Reviewers

Groups

rbtools

People

Description

This change adds a new client for TFS (TFVC). It relies on the presence of the
'tf' command-line tool, which is available either as part of Visual Studio (on
Windows) or via the "Team Explorer Everywhere" package (which is a java
application that can run on just about any platform).

This is intended for use with the TFS SCMTool which is being developed as part
of the Power Pack.

Diffing is done basically by hand, because the 'tf diff[erence]' command is
kind of crummy, even when calling out to GNU Diff via the TF_DIFF_COMMAND
environment variable. In particular, when there are added/deleted files, rather
than including the diff for that file, it just says something like "X is only
present in revision Y". The diffing logic matches that which is used for the
post-commit implementation in the TFS SCMTool.

At the moment, this only supports posting pending changes from the working
directory. There's the beginning of an implementation for committed change
ranges, but when there are moved files, there's no way to match up the orig
file (delete) from the new file (add). I do intend to have it try its best to
post and only give up if a changeset includes more than one moved file, but
that will come later. For now, committed changes can be posted via the web UI.

Testing Done


Posted review requests that had edited files, added files, deleted files,

  renamed files, and moved+edited files.
Verified that binary files were shown correctly.
Ran unit tests.

Issues

Description	From	Last Updated
Col: 80 E501 line too long (86 > 79 characters)	reviewbot	Nov. 25, 2014, 5:14 p.m.
I imagine we'd be impacted by the version number, right? Can we scan for directories with those prefixes? Also, the …	chipx86	Dec. 3, 2014, 4:27 p.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Nov. 25, 2014, 5:14 p.m.
Do we want to break after we find one?	chipx86	Dec. 3, 2014, 4:24 p.m.
No blank line.	chipx86	Dec. 4, 2014, 3:48 p.m.
No blank line.	chipx86	Dec. 4, 2014, 3:48 p.m.
Same.	chipx86	Dec. 4, 2014, 3:48 p.m.
Same.	chipx86	Dec. 4, 2014, 3:49 p.m.
Summary on the first line.	chipx86	Dec. 3, 2014, 4:25 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 25, 2014, 5:14 p.m.
These are unrelated. Blank line here.	chipx86	Dec. 3, 2014, 4:25 p.m.
Same here.	chipx86	Dec. 3, 2014, 4:25 p.m.
We should build as a list and then join, for performance reasons.	chipx86	Dec. 3, 2014, 4:18 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Nov. 25, 2014, 5:14 p.m.
Blank line here.	chipx86	Dec. 3, 2014, 4:26 p.m.
Missing period.	chipx86	Dec. 3, 2014, 4:25 p.m.
We can do as e now, right?	chipx86	Dec. 3, 2014, 4:26 p.m.
Two blank lines.	chipx86	Dec. 3, 2014, 4:26 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Nov. 25, 2014, 5:15 p.m.
Should be one line.	chipx86	Dec. 3, 2014, 4:26 p.m.
I think we've had to do this in a few other places as well. How about we add a get_all() …	chipx86	Dec. 3, 2014, 4:26 p.m.
Col: 80 E501 line too long (87 > 79 characters)	reviewbot	Dec. 4, 2014, 3:31 p.m.
Col: 80 E501 line too long (85 > 79 characters)	reviewbot	Dec. 4, 2014, 3:31 p.m.
Col: 80 E501 line too long (87 > 79 characters)	reviewbot	Dec. 4, 2014, 3:53 p.m.
Col: 80 E501 line too long (85 > 79 characters)	reviewbot	Dec. 4, 2014, 3:53 p.m.

Tool: PEP8 Style Checker
Processed Files:
    rbtools/commands/status.py
    rbtools/commands/setup_repo.py
    rbtools/clients/tfs.py
    rbtools/commands/__init__.py
    rbtools/commands/diff.py
    rbtools/commands/post.py
    setup.py



Tool: Pyflakes
Processed Files:
    rbtools/commands/status.py
    rbtools/commands/setup_repo.py
    rbtools/clients/tfs.py
    rbtools/commands/__init__.py
    rbtools/commands/diff.py
    rbtools/commands/post.py
    setup.py

rbtools/clients/tfs.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (86 > 79 characters)
```
rbtools/clients/tfs.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (84 > 79 characters)
```
rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```

rbtools/clients/tfs.py (Diff revision 1)

The issue has been resolved. Show all issues

I imagine we'd be impacted by the version number, right? Can we scan for directories with those prefixes?

Also, the backslashes are escaping. We probably want r'...'.

david Dec. 3, 2014, 4:27 p.m.

I think so, but I also think MS has a history of changing directory names in unpredictable ways. I think we'd do better to leave this as-is and then see where things are located the next time they release a major version.

rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Do we want to break after we find one?
```

rbtools/clients/tfs.py (Diff revision 1)

The issue has been resolved. Show all issues

No blank line.

david Dec. 3, 2014, 4:27 p.m.

I initally had these this way and I found it was really hard to read, since it's a big conditional.

chipx86 Dec. 4, 2014, 3:42 p.m.

It's inconsistent with all other conditionals throughout RB and RBTools, though. I think it actualy hurts readability, since it looks like an unrelated block of code.

rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
No blank line.
```
rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Same.
```
rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Same.
```
rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Summary on the first line.
```
rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
These are unrelated. Blank line here.
```
rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Same here.
```
rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
We should build as a list and then join, for performance reasons.
```
rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Blank line here.
```
rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Missing period.
```
rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
We can do as e now, right?
```
rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Two blank lines.
```
rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Should be one line.
```

rbtools/clients/tfs.py (Diff revision 1)

The issue has been dropped. Show all issues

I think we've had to do this in a few other places as well. How about we add a get_all() to ListResource that does this?

david Dec. 3, 2014, 4:27 p.m.

I'd prefer to do that cleanup in a separate change.

Commit:

-	17aa7ad97d01201926f2468b9553f9d28c201ad2
+	57ca3bfe12fda7a2b08a531c2221aebfcbbd4b22

Diff:

Revision 2 (+380)

Show changes

	setup.py
	rbtools/clients/tfs.py
	rbtools/commands/__init__.py
	rbtools/commands/diff.py
	rbtools/commands/post.py
	rbtools/commands/setup_repo.py
	rbtools/commands/status.py

Tool: Pyflakes
Processed Files:
    rbtools/commands/status.py
    rbtools/commands/setup_repo.py
    rbtools/clients/tfs.py
    rbtools/commands/__init__.py
    rbtools/commands/diff.py
    rbtools/commands/post.py
    setup.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/commands/status.py
    rbtools/commands/setup_repo.py
    rbtools/clients/tfs.py
    rbtools/commands/__init__.py
    rbtools/commands/diff.py
    rbtools/commands/post.py
    setup.py

rbtools/clients/tfs.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (87 > 79 characters)
```
rbtools/clients/tfs.py (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (85 > 79 characters)
```

Change Summary:

Whitespace.

Commit:

-	57ca3bfe12fda7a2b08a531c2221aebfcbbd4b22
+	a34f92e78376f734011162c10060f74a8d7a401d

Diff:

Revision 3 (+376)

Show changes

	setup.py
	rbtools/clients/tfs.py
	rbtools/commands/__init__.py
	rbtools/commands/diff.py
	rbtools/commands/post.py
	rbtools/commands/setup_repo.py
	rbtools/commands/status.py

Tool: PEP8 Style Checker
Processed Files:
    rbtools/commands/status.py
    rbtools/commands/setup_repo.py
    rbtools/clients/tfs.py
    rbtools/commands/__init__.py
    rbtools/commands/diff.py
    rbtools/commands/post.py
    setup.py



Tool: Pyflakes
Processed Files:
    rbtools/commands/status.py
    rbtools/commands/setup_repo.py
    rbtools/clients/tfs.py
    rbtools/commands/__init__.py
    rbtools/commands/diff.py
    rbtools/commands/post.py
    setup.py

rbtools/clients/tfs.py (Diff revision 3)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (87 > 79 characters)
```
rbtools/clients/tfs.py (Diff revision 3)
The issue has been dropped. Show all issues
```
Col: 80
 E501 line too long (85 > 79 characters)
```

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to master (864884b)