Summary: |
|
|---|
Add support for reading and writing history metadata
Review Request #8278 — Created July 11, 2016 and submitted
For server-side detection of rebases and commit amends, we need to keep
track of some metadata indicating the original SHA1 of commits. With
git, we can store this metadata ingit notes.
git notesallows for storing arbitrary metadata associated with each
commit. We store the SHA1 of the commit in a special notes ref,
rbt-history-metadata. This way, we do not interfere with notes that
users may be using. We also add this ref to the list of refs to be
rewritten during rebases and amends.When an amend occurs, the SHA1 of the commit will change, but the
stored SHA1 will not, so we will be able to detect that commit was
amended. Likewise, when a squash occurs during a rebase, the SHA1 will
change but the stored metadata will be the SHA1 hashes of all the
squashed commits, so we can also know when a squash occurs. We can then
upload this metadata when doingrbt postso that Review Board can
detect history changes server side.
Using https://reviews.reviewboard.org/r/8276/, I was able to do the
following:
- Upload a review request with multiple commits.
- Squash the commits together locally.
- Update the review request.
I observed that the original commits were set in the Review Board
database.
| Description | From | Last Updated |
|---|---|---|
|
Can we make this if commits is None instead? |
 david
|
|
|
Can we rearrange and simplify this a tiny bit? I don't think we need separate error messages for the two … |
david
|
|
|
Instead of the early return, I think it would be better to put the logging statement into the conditional and … |
david
|
Testing Done: |
|
|---|
-
-
-
rbtools/clients/git.py (Diff revision 1) Can we rearrange and simplify this a tiny bit? I don't think we need separate error messages for the two cases because the remedy is the same:
if rewrite_mode.strip() != 'concatenate': logging.warn('notes.rewriteMode is not set to concatenate. ' 'Review Board will not be able to correctly track ' 'rebased and amended commits without this. To fix ' 'this, run:\n' '\tgit config notes.rewriteMode concatenate')
-
Tool: Pyflakes Processed Files: rbtools/commands/post.py rbtools/clients/__init__.py rbtools/api/resource.py rbtools/clients/git.py Tool: PEP8 Style Checker Processed Files: rbtools/commands/post.py rbtools/clients/__init__.py rbtools/api/resource.py rbtools/clients/git.py
-
-
rbtools/clients/git.py (Diff revision 2) Instead of the early return, I think it would be better to put the logging statement into the conditional and just let control flow go all the way to the end of the function. This is simplest if you don't use
none_on_ignored_errorin theexecutecall, in which case you should always have a string, but you could also test forrewrite_mode is None or rewrite_mode.strip() != 'concatenate'
Tool: Pyflakes Processed Files: rbtools/commands/post.py rbtools/clients/__init__.py rbtools/api/resource.py rbtools/clients/git.py Tool: PEP8 Style Checker Processed Files: rbtools/commands/post.py rbtools/clients/__init__.py rbtools/api/resource.py rbtools/clients/git.py