Merge ~bjornt/gitlptools:fix-merge-target into ~ubuntuone-hackers/gitlptools:master

Proposed by Björn Tillenius
Status: Merged
Approved by: Adam Collard
Approved revision: 3ad3b82c05c24794587af7d2a006a1f7cb4cc6dd
Merged at revision: db68ca6ecfe103df187315a6439ad49122c88b28
Proposed branch: ~bjornt/gitlptools:fix-merge-target
Merge into: ~ubuntuone-hackers/gitlptools:master
Diff against target: 83 lines (+18/-9)
2 files modified
src/gitlptools/__init__.py (+12/-4)
tests/test_gitlptools.py (+6/-5)
Reviewer Review Type Date Requested Status
Adam Collard (community) Approve
Review via email: mp+354400@code.launchpad.net

Commit message

Don't require people to use lp: URLs.

The code that found out the merge target in order to generate
the diff assumed that the remote URL started with 'lp:'.

Now it works even if you don't set up that alias, and instead
keep the original git+ssh://git.launchpad.net/ URLs.

To post a comment you must log in.
Revision history for this message
Adam Collard (adam-collard) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/gitlptools/__init__.py b/src/gitlptools/__init__.py
2index f9c36d4..8747895 100644
3--- a/src/gitlptools/__init__.py
4+++ b/src/gitlptools/__init__.py
5@@ -204,6 +204,9 @@ def git_lp_propose(argv=sys.argv[1:]):
6 'configs for the branch, if set.'))
7 parser.add_argument(
8 '-m', '--message', help='Commit message for the merge proposal.')
9+ parser.add_argument(
10+ '--dry-run', help="Don't actually create the merge proposal.",
11+ action='store_true', default=False)
12 options = parse_args_with_logging(parser, argv)
13 logger = logging.getLogger('gitlptools')
14 merge_target = options.merge_target
15@@ -239,6 +242,10 @@ def git_lp_propose(argv=sys.argv[1:]):
16 if not commit_message:
17 sys.exit("Empty commit message. Aborted!")
18
19+ if options.dry_run:
20+ logger.info('Dry run. Aborted!')
21+ sys.exit(0)
22+
23 try:
24 mp = lp_target_refs.branch.createMergeProposal(
25 merge_target=lp_target_refs.target, commit_message=commit_message,
26@@ -329,10 +336,11 @@ def get_lp_target_refs(lp, lp_git_ref, merge_target):
27 branch=lp_user_repo.getRefByPath(path=lp_git_ref.lp_branch_name))
28
29
30-def get_remote_urls_map(git_repo):
31- """Return a dict mapping remote urls to their names."""
32+def get_remote_paths_map(git_repo):
33+ """Return a dict mapping remote paths to their names."""
34 return {
35- remote.url.lstrip('lp:'): remote.name for remote in git_repo.remotes}
36+ get_launchpad_path(remote.url): remote.name
37+ for remote in git_repo.remotes}
38
39
40 def prompt_for_commit_message(git_repo, merge_target):
41@@ -363,7 +371,7 @@ def prompt_for_commit_message(git_repo, merge_target):
42 def _get_commit_messages_and_stats(git_repo, merge_target):
43 """Return with the"""
44 try:
45- target = get_remote_urls_map(git_repo)[merge_target.repo]
46+ target = get_remote_paths_map(git_repo)[merge_target.repo]
47 except KeyError:
48 raise UnknownTargetURL(merge_target.repo)
49
50diff --git a/tests/test_gitlptools.py b/tests/test_gitlptools.py
51index 19298fb..6cc0fef 100644
52--- a/tests/test_gitlptools.py
53+++ b/tests/test_gitlptools.py
54@@ -10,7 +10,7 @@ from gitlptools import (
55 InvalidMergeTarget,
56 LPGitRef,
57 parse_args_with_logging,
58- get_remote_urls_map
59+ get_remote_paths_map
60 )
61
62
63@@ -139,15 +139,16 @@ def test_merge_target_remote_unknown():
64 get_merge_target(git_repo, "unknown/master")
65
66
67-def test_get_remote_urls_map():
68+@pytest.mark.parametrize('url_prefix', ['lp:', 'git+ssh://git.launchpad.net/'])
69+def test_get_remote_paths_map(url_prefix):
70 git_repo = Mock(
71 remotes=[
72- FakeRemote(name='upstream', url='lp:testrepo'),
73- FakeRemote(name='origin', url='lp:~user/testrepo')])
74+ FakeRemote(name='upstream', url=url_prefix + 'testrepo'),
75+ FakeRemote(name='origin', url=url_prefix + '~user/testrepo')])
76 expected = {
77 'testrepo': 'upstream',
78 '~user/testrepo': 'origin'}
79- assert get_remote_urls_map(git_repo) == expected
80+ assert get_remote_paths_map(git_repo) == expected
81
82
83 def test_parse_args_with_logging_no_args():

Subscribers

People subscribed via source and target branches