Looks good, I just ask for a couple comments for the refs fetch and push, and a couple minor inline notes. Otherwise looks solid, some really good refactoring, and I'm looking forward to seeing the new notes feature in practice. I'm still a bit nervous that this will uncover warts in git, but maybe not, and in any case it feels like the right way to go technically. If you agree with the comment suggestions, please go ahead and land after adding, no need for second round of review. * Review √ python3 setup.py build √ python3 setup.py check √ ./snap-wrappers/wrappers/git-ubuntu-self-test No broken requirements found. pip3 found all required dependencies pylint passed! PASS (syntax) source-package-walker.py PASS (compilation) source-package-walker.py PASS (syntax) update-repository-alias.py PASS (compilation) update-repository-alias.py PASS (syntax) import-source-packages.py PASS (compilation) import-source-packages.py 332 passed, 3 xfailed in 187.90 seconds ! Per-patch code review √ ef26b8a077b6ff3a8c455d38fc838dad4625df00 + Die wrappers! + LGTM √ 4bdd58db60bad113b1eb6a87621554f8f8bedef6 + Pythonic objects FTW + LGTM √ 69bc94052dbd1793fce7f4f9f32b76cb2c8f3b98 + Verified the promised skip of test_get_import_commit_msg() is indeed dropped in 061371a7 (next patch). + LGTM √ 061371a77089930b27a261ff0447ba60d517fa82 + This is the key patch in the set. Moves change info from the commit message into the git note. + add_changelog_note_to_commit() is added, and has corresponding test cases test_add_changelog_note_to_commit() and test_double_changelog_note_add_does_not_fail() + Uses the new gitubuntu.spec module + LGTM √ 224684300a41a929492f981c453a3f71d77175d6 + Largely cleanup/refactor from prior commit. Uses patch_state. + LGTM, nice cleanup √ 9323899deab7a1d57e3c0ac9f92a47f552602502 + Verified all call points are updated with correct argument list. + LGTM √ 37ea80888ea65deaeeb509e3624e2a0f6b92d61c + Verified no consumers for _commit_import() other than importer.py, and all call points are updated with correct argument list. + LGTM ! a133d0da5316cedf72c220649ffa8691057a3c2c + If I understand the refspecs properly, then the fetch with this refspec: 'refs/notes/commits:refs/notes/usd-import-team/changelog' will import the notes from 'refs/notes/commits' in the remote repository, into 'refs/notes/usd-import-team/changelog' in the local checkout. Then later, there is a repo.git_run() call that pushes them back in the opposite direction. + The commit message explains this sufficiently, but the _main_with_repo() itself is lengthy (250+ lines!) and only lightly commented, so this notes pushing/pulling feature may not be clear to future readers. This would be a good opportunity to add a comment each for the repo.fetch_remote_refspecs() and git_run() calls, to define the refspec's being fetched and pulled. Hopefully over time this routine can be whittled down, and these comments will eventually turn into proper codedocs somewhere. + The commit message suggests this is a workaround to a limitation in Launchpad that is anticipated to be fixed. If this is the case please also include: • Link to the relevant LP bug or discussion in commit msg • Comment in the code to flag when the refs/notes/commits can be dropped • Bug report against usd-importer to follow up on the tech debt √ b0319c9f18629d6548988ac93ea9d98f1f4a5bf2 + LGTM √ 49d62ab97b7107abfb3dc2a993b50a74cf79215b + LGTM