Code review comment for ~racb/git-ubuntu:commit-message-spec

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