Merge lp:~cjwatson/launchpad/git-optimise-empty-commits into lp:launchpad

Proposed by Colin Watson
Status: Merged
Merged at revision: 18736
Proposed branch: lp:~cjwatson/launchpad/git-optimise-empty-commits
Merge into: lp:launchpad
Diff against target: 30 lines (+9/-0)
2 files modified
lib/lp/code/model/gitrepository.py (+2/-0)
lib/lp/code/model/tests/test_gitrepository.py (+7/-0)
To merge this branch: bzr merge lp:~cjwatson/launchpad/git-optimise-empty-commits
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+350725@code.launchpad.net

Commit message

Optimise GitRepository.fetchRefCommits if there are no commits to fetch.

Description of the change

This seems rather common in practice (e.g. a push that just deletes refs), so we might as well save a request.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/model/gitrepository.py'
2--- lib/lp/code/model/gitrepository.py 2018-07-13 09:17:40 +0000
3+++ lib/lp/code/model/gitrepository.py 2018-07-24 12:25:11 +0000
4@@ -676,6 +676,8 @@
5 def fetchRefCommits(hosting_path, refs, logger=None):
6 """See `IGitRepository`."""
7 oids = sorted(set(info["sha1"] for info in refs.values()))
8+ if not oids:
9+ return
10 commits = parse_git_commits(
11 getUtility(IGitHostingClient).getCommits(
12 hosting_path, oids, logger=logger))
13
14=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
15--- lib/lp/code/model/tests/test_gitrepository.py 2017-11-06 09:32:45 +0000
16+++ lib/lp/code/model/tests/test_gitrepository.py 2018-07-24 12:25:11 +0000
17@@ -1337,6 +1337,13 @@
18 }
19 self.assertEqual(expected_refs, refs)
20
21+ def test_fetchRefCommits_empty(self):
22+ # If given an empty refs dictionary, fetchRefCommits returns early
23+ # without contacting the hosting service.
24+ hosting_fixture = self.useFixture(GitHostingFixture())
25+ GitRepository.fetchRefCommits("dummy", {})
26+ self.assertEqual([], hosting_fixture.getCommits.calls)
27+
28 def test_synchroniseRefs(self):
29 # synchroniseRefs copes with synchronising a repository where some
30 # refs have been created, some deleted, and some changed.