1357236 - Extract commit metadata rewriting to own function

Status: RESOLVED FIXED

(Developer Services :: Servo VCS Sync, enhancement)

Description

[Gregory Szorc [:gps]]

Will be explained in commit message.

Comment 1

[Gregory Szorc [:gps]]

Attachment: vcssync: rename variables so it is clear they store paths;

An upcoming commit will introduce a dulwich.repo.Repo into this
function. To reduce confusion as to what variables refer to, this
commit explicitly adds "path" to variables that hold paths.

Review commit: https://reviewboard.mozilla.org/r/130982/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/130982/

Comment 2

[Gregory Szorc [:gps]]

Attachment: vcssync: extract git commit rewriting to standalone function (bug 1357236);

linearize_git_repo() accepts a bunch of arguments and contains code
for rewriting Git commit object metadata (author field, summary line,
commit message, etc). This rewriting is not critical to the operation
of this function. It exists there for hysterical raisins.

As the inline TODO in that function states, we want to extract this
metadata rewriting to its own function so we have separation of
concerns and can perform similar rewriting from other consumers
(such as a Git pull request importer).

This commit extracts the Git commit metadata rewriting to its own
reusable function.

The new commit_metadata_rewriter() function accepts a repo and
behavioral arguments and returns a function that is called to
process a pair of old and new Git commit objects. This function
is then passed to linearize_git_repo() and called on each processed
commit.

This change is a bit larger than I would have liked. There was
significant churn around callers of linearize_git_repo(), including
from the CLI layer. Some of this was due to needing to construct a
dulwich.repo.Repo instance. Other changes were around handling of
argument passing. The new code relies much less on **kwargs and
I think that is a good thing because it is too easy for **kwargs
to hide subtle bugs like unused arguments. Unfortunately, it does
make the change a bit harder to review.

Review commit: https://reviewboard.mozilla.org/r/130984/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/130984/

Comment 3

[Gregory Szorc [:gps]]

Comment on attachment 8858978 [details]
vcssync: extract git commit rewriting to standalone function (bug 1357236);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/130984/diff/1-2/

Comment 4

[:glob ✱]

Comment on attachment 8858977 [details]
vcssync: rename variables so it is clear they store paths;

https://reviewboard.mozilla.org/r/130982/#review134182

Comment 5

[:glob ✱]

Comment on attachment 8858978 [details]
vcssync: extract git commit rewriting to standalone function (bug 1357236);

https://reviewboard.mozilla.org/r/130984/#review134184

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/86c6e05ed52c
vcssync: extract git commit rewriting to standalone function ; r=glob