From 03768832c4a73c3079ce24d84fecae1da39b2d71 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 15 Oct 2018 14:20:41 +0100 Subject: [PATCH] Remove auto-branch name The feature to configure branch names based on the presence of "bug", "lp", "blueprint" or "bp" in the commit message is overly elaborate and very OpenStack specific. Even with this, it hasn't been updated to keep up with the times as many projects have migrated to Storyboard, which isn't handled here. Given that it frequently does the wrong thing and likely doesn't apply to anyone outside of the OpenStack ecosystem, the wisest thing it to simply remove the feature. This is a break in behavior but it seems better than adding yet another flag for something that many users will want enabled by default. Change-Id: I82ecc1719de5c87d59bbfe73a042917e6559da1e Signed-off-by: Stephen Finucane Story: 1130330 Task: 566 Story: 2001247 Task: 5777 --- git_review/cmd.py | 58 ++----------- git_review/tests/test_git_review.py | 85 +------------------ ...omatic-branch-naming-8e3e2f6487637b86.yaml | 10 +++ 3 files changed, 20 insertions(+), 133 deletions(-) create mode 100644 releasenotes/notes/remove-automatic-branch-naming-8e3e2f6487637b86.yaml diff --git a/git_review/cmd.py b/git_review/cmd.py index 99db6262..7f6e444d 100644 --- a/git_review/cmd.py +++ b/git_review/cmd.py @@ -938,61 +938,21 @@ def assert_one_change(remote, branch, yes, have_hook): sys.exit(1) -def use_topic(why, topic): - """Inform the user about why a particular topic has been selected.""" - if VERBOSE: - print(why % ('"%s"' % topic,)) - return topic - - def get_topic(target_branch): - branch_name = get_branch_name(target_branch) branch_parts = branch_name.split("/") if len(branch_parts) >= 3 and branch_parts[0] == "review": - return use_topic("Using change number %s " - "for the topic of the change submitted", - "/".join(branch_parts[2:])) + topic = "/".join(branch_parts[2:]) + if VERBOSE: + print("Using change number %s for the topic of the change " + "submitted" % topic) + return topic - preferred_log_format = "%B" - log_output = run_command("git log --pretty='" + preferred_log_format + - "' HEAD^1..HEAD") - if log_output == preferred_log_format: - # The %B format specifier is supported starting at Git v1.7.2. If it's - # not supported, we'll just get back '%B', so we try something else. - # The downside of %s is that it removes newlines in the subject. - log_output = run_command("git log --pretty='%s%n%b' HEAD^1..HEAD") - bug_re = r'''(?x) # verbose regexp - \b([Bb]ug|[Ll][Pp]) # bug or lp - [ \t\f\v]* # don't want to match newline - [:]? # separator if needed - [ \t\f\v]* # don't want to match newline - [#]? # if needed - [ \t\f\v]* # don't want to match newline - (\d+) # bug number''' - - match = re.search(bug_re, log_output) - if match is not None: - return use_topic("Using bug number %s " - "for the topic of the change submitted", - "bug/%s" % match.group(2)) - - bp_re = r'''(?x) # verbose regexp - \b([Bb]lue[Pp]rint|[Bb][Pp]) # a blueprint or bp - [ \t\f\v]* # don't want to match newline - [#:]? # separator if needed - [ \t\f\v]* # don't want to match newline - ([0-9a-zA-Z-_]+) # any identifier or number''' - match = re.search(bp_re, log_output) - if match is not None: - return use_topic("Using blueprint number %s " - "for the topic of the change submitted", - "bp/%s" % match.group(2)) - - return use_topic("Using local branch name %s " - "for the topic of the change submitted", - branch_name) + if VERBOSE: + print("Using local branch name %s for the topic of the change " + "submitted" % branch_name) + return branch_name class CannotQueryOpenChangesets(CommandFailed): diff --git a/git_review/tests/test_git_review.py b/git_review/tests/test_git_review.py index fe778f88..a02f44d7 100644 --- a/git_review/tests/test_git_review.py +++ b/git_review/tests/test_git_review.py @@ -389,92 +389,9 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): self._assert_branch_would_be('master%topic=zat', extra_args=['-t', 'zat']) - def test_bug_topic(self): - self._run_git_review('-s') - self._simple_change('a change', 'new change for bug 123') - self._assert_branch_would_be('master%topic=bug/123') - - def test_bug_topic_newline(self): - self._run_git_review('-s') - self._simple_change('a change', 'new change not for bug\n\n123') - self._assert_branch_would_be('master') - - def test_bp_topic(self): - self._run_git_review('-s') - self._simple_change('a change', 'new change for blueprint asdf') - self._assert_branch_would_be('master%topic=bp/asdf') - - def test_bp_topic_newline(self): - self._run_git_review('-s') - self._simple_change('a change', 'new change not for blueprint\n\nasdf') - self._assert_branch_would_be('master') - - def test_author_name_topic_bp(self): - old_author = None - if 'GIT_AUTHOR_NAME' in os.environ: - old_author = os.environ['GIT_AUTHOR_NAME'] - try: - os.environ['GIT_AUTHOR_NAME'] = 'BPNAME' - self._run_git_review('-s') - self._simple_change('a change', - 'new change 1 with name but no topic') - self._assert_branch_would_be('master') - finally: - if old_author: - os.environ['GIT_AUTHOR_NAME'] = old_author - else: - del os.environ['GIT_AUTHOR_NAME'] - - def test_author_email_topic_bp(self): - old_author = None - if 'GIT_AUTHOR_EMAIL' in os.environ: - old_author = os.environ['GIT_AUTHOR_EMAIL'] - try: - os.environ['GIT_AUTHOR_EMAIL'] = 'bpemail@example.com' - self._run_git_review('-s') - self._simple_change('a change', - 'new change 1 with email but no topic') - self._assert_branch_would_be('master') - finally: - if old_author: - os.environ['GIT_AUTHOR_EMAIL'] = old_author - else: - del os.environ['GIT_AUTHOR_EMAIL'] - - def test_author_name_topic_bug(self): - old_author = None - if 'GIT_AUTHOR_NAME' in os.environ: - old_author = os.environ['GIT_AUTHOR_NAME'] - try: - os.environ['GIT_AUTHOR_NAME'] = 'Bug: #1234' - self._run_git_review('-s') - self._simple_change('a change', - 'new change 2 with name but no topic') - self._assert_branch_would_be('master') - finally: - if old_author: - os.environ['GIT_AUTHOR_NAME'] = old_author - else: - del os.environ['GIT_AUTHOR_NAME'] - - def test_author_email_topic_bug(self): - old_author = None - if 'GIT_AUTHOR_EMAIL' in os.environ: - old_author = os.environ['GIT_AUTHOR_EMAIL'] - try: - os.environ['GIT_AUTHOR_EMAIL'] = 'bug5678@example.com' - self._run_git_review('-s') - self._simple_change('a change', - 'new change 2 with email but no topic') - self._assert_branch_would_be('master') - finally: - if old_author: - os.environ['GIT_AUTHOR_EMAIL'] = old_author - else: - del os.environ['GIT_AUTHOR_EMAIL'] - def test_git_review_T(self): self._run_git_review('-s') + self._run_git('checkout', '-b', 'bug/456') self._simple_change('test file modified', 'commit message for bug 456') self._assert_branch_would_be('master%topic=bug/456') self._assert_branch_would_be('master', extra_args=['-T']) diff --git a/releasenotes/notes/remove-automatic-branch-naming-8e3e2f6487637b86.yaml b/releasenotes/notes/remove-automatic-branch-naming-8e3e2f6487637b86.yaml new file mode 100644 index 00000000..8258a4d2 --- /dev/null +++ b/releasenotes/notes/remove-automatic-branch-naming-8e3e2f6487637b86.yaml @@ -0,0 +1,10 @@ +--- +upgrade: + - | + Support for auto-configuration of topic names based on the presence of + keywords in the commit message is removed. Previously, keywords such as + ``bug``, ``bp`` or ``lp`` followed by a number would result in branch + names containing the associated number, e.g. ``bug/123``. This feature + was OpenStack specific, was a frequent source of bugs and clearly wasn't + being used that often, given that StoryBoard is a thing now and no one had + updated this feature to handle that metadata.