- Change Summary:
-
Do some refactoring:
- Split the file extraction code out into their own methods, to prevent the diff method from becoming too huge.
- Rename some things to be more consistent.
- Handle the base revision for added files in a better way.
Implement posting for shelved perforce changes.
Review Request #5172 — Created Dec. 27, 2013 and submitted
Implement posting for shelved perforce changes.
The perforce shelf is kind of like a shared "git stash". When you shelve a
change, it copies the data from your working directory over to the server. What
I didn't expect is that it also leaves your working directory the same, with
the same open files (which can then be reverted).We can get the file list for a shelved change via `
p4 files //...@=CLN
`, and
the file contents via `p4 print //path/to/file@=CLN
`. This lets us use a very
similar implementation to pending changes, with the exception that the "new"
files are fetched from the repository instead of just using the working copy.The perforce documentation is full of lies. Among other fantastical claims, it
asserts that shelved changes have a 'Status' field of 'shelved', when in
practice, they're still 'pending'. I've made changes to parse_revision_spec to
treat shelved changes the same as pending ones, and then made some changes to
the pending diff code path to look on the shelf if there are no open files in
the changeset.The one major limitation that I wasn't able to work around is that when you
shelve a change containing file moves, there doesn't seem to be any way to know
which "move/add" corresponds to which "move/delete". The usual ways of doing
this (looking for "movedFile" in fstat, or looking in the filelog) don't work
for shelved changesets. I suspect that this is just an oversight that will be
corrected in the course of time, but for now, I've made it so that when you
post such a change, you'll see an old-school deleted file and an added file,
even when the Review Board server supports moved files.
- Posted a variety of shelved changes with different types of changes contained
within. - Ran
rbt diff N
with a shelved changeset, which is what prompted the addition
of the options to that command. - Ran unit tests.
-
-
I think you need the
::
right up againstnote
. At least, we do that everywhere else, so it should be consistent. -
-
This seems pretty similar to
_extract_add_files
. Maybe you could do:def _extract_edit_files(...): old_filename, new_filename = self._extract_add_files(...) self._write_file(..., old_filename) return old_filename, new_filename
No big deal either way.