kou commented on code in PR #14750:
URL: https://github.com/apache/arrow/pull/14750#discussion_r1036421443
##########
dev/merge_arrow_pr.py:
##########
@@ -187,71 +156,177 @@ def _filter_mainline_versions(self, versions):
return [x for x in versions if mainline_regex.match(x.name)]
- def resolve(self, fix_versions, comment):
+ def resolve(self, fix_version, comment):
fields = self.issue.fields
cur_status = fields.status.name
if cur_status == "Resolved" or cur_status == "Closed":
self.cmd.fail("JIRA issue %s already has status '%s'"
% (self.jira_id, cur_status))
- if DEBUG:
- print("JIRA issue %s untouched" % (self.jira_id))
- return
-
resolve = [x for x in self.jira_con.transitions(self.jira_id)
if x['name'] == "Resolve Issue"][0]
# ARROW-6915: do not overwrite existing fix versions corresponding to
# point releases
- fix_versions = list(fix_versions)
+ fix_versions = [v.raw for v in self.jira_con.project_versions(
+ self.project) if v.name == fix_version]
fix_version_names = set(x['name'] for x in fix_versions)
for version in self.current_fix_versions:
major, minor, patch = version.name.split('.')
if patch != '0' and version.name not in fix_version_names:
fix_versions.append(version.raw)
- self.jira_con.transition_issue(self.jira_id, resolve["id"],
- comment=comment,
- fixVersions=fix_versions)
-
- print("Successfully resolved %s!" % (self.jira_id))
+ if DEBUG:
+ print("JIRA issue %s untouched -> %s" %
+ (self.jira_id, [v["name"] for v in fix_versions]))
+ else:
+ self.jira_con.transition_issue(self.jira_id, resolve["id"],
+ comment=comment,
+ fixVersions=fix_versions)
+ print("Successfully resolved %s!" % (self.jira_id))
self.issue = self.jira_con.issue(self.jira_id)
self.show()
def show(self):
fields = self.issue.fields
- print(format_jira_output(self.jira_id, fields.status.name,
- fields.summary, fields.assignee,
- fields.components))
+ print(format_issue_output("jira", self.jira_id, fields.status.name,
+ fields.summary, fields.assignee,
+ fields.components))
-def format_jira_output(jira_id, status, summary, assignee, components):
- if assignee is None:
+class GitHubIssue(object):
+
+ def __init__(self, github_api, github_id, cmd):
+ self.github_api = github_api
+ self.github_id = github_id
+ self.cmd = cmd
+
+ try:
+ self.issue = self.github_api.get_issue_data(github_id)
+ except Exception as e:
+ self.cmd.fail("Github could not find %s\n%s" % (github_id, e))
Review Comment:
```suggestion
self.cmd.fail("GitHub could not find %s\n%s" % (github_id, e))
```
##########
dev/merge_arrow_pr.py:
##########
@@ -366,24 +491,25 @@ def maintenance_branches(self):
return [x["name"] for x in self._github_api.get_branches()
if x["name"].startswith("maint-")]
- def _get_jira(self):
+ def _get_issue(self):
if self.title.startswith("MINOR:"):
return None
- jira_id = None
- for project, regex in PR_TITLE_REGEXEN:
+ m = self.GITHUB_PR_TITLE_REGEXEN.search(self.title)
+ if m:
+ github_id = m.group(1)
+ return GitHubIssue(self._github_api, github_id, self.cmd)
+
+ for project, regex in self.JIRA_PR_TITLE_REGEXEN:
m = regex.search(self.title)
if m:
jira_id = m.group(1)
- break
-
- if jira_id is None:
- options = ' or '.join('{0}-XXX'.format(project)
- for project in SUPPORTED_PROJECTS)
- self.cmd.fail("PR title should be prefixed by a jira id "
- "{0}, but found {1}".format(options, self.title))
+ return JiraIssue(self.con, jira_id, project, self.cmd)
- return JiraIssue(self.con, jira_id, project, self.cmd)
+ options = ' or '.join('{0}-XXX'.format(project)
+ for project in self.JIRA_SUPPORTED_PROJECTS)
+ self.cmd.fail("PR title should be prefixed by a jira id "
Review Comment:
```suggestion
self.cmd.fail("PR title should be prefixed by a GitHub ID or a Jira
ID "
```
##########
dev/merge_arrow_pr.py:
##########
@@ -187,71 +156,177 @@ def _filter_mainline_versions(self, versions):
return [x for x in versions if mainline_regex.match(x.name)]
- def resolve(self, fix_versions, comment):
+ def resolve(self, fix_version, comment):
fields = self.issue.fields
cur_status = fields.status.name
if cur_status == "Resolved" or cur_status == "Closed":
self.cmd.fail("JIRA issue %s already has status '%s'"
% (self.jira_id, cur_status))
- if DEBUG:
- print("JIRA issue %s untouched" % (self.jira_id))
- return
-
resolve = [x for x in self.jira_con.transitions(self.jira_id)
if x['name'] == "Resolve Issue"][0]
# ARROW-6915: do not overwrite existing fix versions corresponding to
# point releases
- fix_versions = list(fix_versions)
+ fix_versions = [v.raw for v in self.jira_con.project_versions(
+ self.project) if v.name == fix_version]
fix_version_names = set(x['name'] for x in fix_versions)
for version in self.current_fix_versions:
major, minor, patch = version.name.split('.')
if patch != '0' and version.name not in fix_version_names:
fix_versions.append(version.raw)
- self.jira_con.transition_issue(self.jira_id, resolve["id"],
- comment=comment,
- fixVersions=fix_versions)
-
- print("Successfully resolved %s!" % (self.jira_id))
+ if DEBUG:
+ print("JIRA issue %s untouched -> %s" %
+ (self.jira_id, [v["name"] for v in fix_versions]))
+ else:
+ self.jira_con.transition_issue(self.jira_id, resolve["id"],
+ comment=comment,
+ fixVersions=fix_versions)
+ print("Successfully resolved %s!" % (self.jira_id))
self.issue = self.jira_con.issue(self.jira_id)
self.show()
def show(self):
fields = self.issue.fields
- print(format_jira_output(self.jira_id, fields.status.name,
- fields.summary, fields.assignee,
- fields.components))
+ print(format_issue_output("jira", self.jira_id, fields.status.name,
+ fields.summary, fields.assignee,
+ fields.components))
-def format_jira_output(jira_id, status, summary, assignee, components):
- if assignee is None:
+class GitHubIssue(object):
+
+ def __init__(self, github_api, github_id, cmd):
+ self.github_api = github_api
+ self.github_id = github_id
+ self.cmd = cmd
+
+ try:
+ self.issue = self.github_api.get_issue_data(github_id)
+ except Exception as e:
+ self.cmd.fail("Github could not find %s\n%s" % (github_id, e))
+
+ def get_label(self, prefix):
+ prefix = f"{prefix}:"
+ return [
+ lbl["name"][len(prefix):].strip()
+ for lbl in self.issue["labels"] if lbl["name"].startswith(prefix)
+ ]
+
+ @property
+ def components(self):
+ return self.get_label("Component")
+
+ @property
+ def assignees(self):
+ return [a["login"] for a in self.issue["assignees"]]
+
+ @property
+ def current_fix_versions(self):
+ return self.issue.get("milestone", {}).get("title")
+
+ @property
+ def current_versions(self):
+ all_versions = self.github_api.get_milestones()
+
+ unreleased_versions = [x for x in all_versions if x["state"] == "open"]
+ unreleased_versions = [x["title"] for x in unreleased_versions]
+
+ return unreleased_versions
+
+ def resolve(self, fix_version, comment):
+ cur_status = self.issue["state"]
+
+ if cur_status == "closed":
+ self.cmd.fail("GitHub issue %s already has status '%s'"
+ % (self.github_id, cur_status))
+
+ if DEBUG:
+ print("GitHub issue %s untouched -> %s" %
+ (self.github_id, fix_version))
+ else:
+ self.github_api.assign_milestone(self.github_id, fix_version)
+ self.github_api.close_issue(self.github_id, comment)
Review Comment:
We may not need this because we can use GitHub's auto close feature by
`close GH-XXX` commit message.
##########
dev/merge_arrow_pr.py:
##########
@@ -32,8 +32,8 @@
# Configuration environment variables:
# - APACHE_JIRA_TOKEN: your Apache JIRA Personal Access Token
# - ARROW_GITHUB_API_TOKEN: a GitHub API token to use for API requests
-# - PR_REMOTE_NAME: the name of the remote to the Apache git repo (set to
-# 'apache' by default)
+# - ORG_NAME: the name of the remote to the Apache git repo
Review Comment:
How about `ARROW_GITHUB_ORG` or `ARROW_GITHUB_OWNER` because
`ARROW_GITHUB_API_TOKEN` has `ARROW_GITHUB_` prefix?
> the remote to the Apache git repo
Could you also update this description?
##########
dev/merge_arrow_pr.py:
##########
@@ -322,6 +440,13 @@ def continue_maybe(self, prompt):
class PullRequest(object):
+ # We can merge both ARROW and PARQUET patches
+ GITHUB_PR_TITLE_REGEXEN = re.compile(r'^GH-([0-9]+)\b.*$')
Review Comment:
I think that `REGEXEN` meant "N regular expressions". (I think that "EXE" is
a typo of "EXP" or `EX`.)
This is just one regular expression. How about `GITHUB_PR_TITLE_REGEX` or
`GITHUB_PR_TITLE_PATTERN`?
##########
dev/test_merge_arrow_pr.py:
##########
@@ -332,9 +346,10 @@ def test_jira_output_no_components():
def test_sorting_versions():
versions_json = [
+ {'name': '11.0.0', 'released': False},
{'name': '9.0.0', 'released': False},
{'name': '10.0.0', 'released': False},
]
versions = [FakeVersion(raw['name'], raw) for raw in versions_json]
- ordered_versions = merge_arrow_pr.JiraIssue.sort_versions(versions)
- assert ordered_versions[0].name == "10.0.0"
+ fix_version = merge_arrow_pr.get_candidate_fix_version(versions)
+ assert fix_version == "9.0.0"
Review Comment:
Do we want to add tests for `GitHubIssue` like existing `JiraIssue` tests?
##########
dev/merge_arrow_pr.py:
##########
@@ -322,6 +440,13 @@ def continue_maybe(self, prompt):
class PullRequest(object):
+ # We can merge both ARROW and PARQUET patches
+ GITHUB_PR_TITLE_REGEXEN = re.compile(r'^GH-([0-9]+)\b.*$')
+ JIRA_SUPPORTED_PROJECTS = ['ARROW', 'PARQUET']
Review Comment:
```suggestion
GITHUB_PR_TITLE_REGEXEN = re.compile(r'^GH-([0-9]+)\b.*$')
# We can merge both ARROW and PARQUET patches
JIRA_SUPPORTED_PROJECTS = ['ARROW', 'PARQUET']
```
##########
dev/merge_arrow_pr.py:
##########
@@ -487,25 +613,19 @@ def get_primary_author(cmd, distinct_authors):
return primary_author, distinct_other_authors
-def prompt_for_fix_version(cmd, jira_issue, maintenance_branches=()):
- (all_versions,
- default_fix_versions) = jira_issue.get_candidate_fix_versions(
+def prompt_for_fix_version(cmd, issue, maintenance_branches=()):
+ default_fix_version = get_candidate_fix_version(
+ issue.current_versions,
maintenance_branches=maintenance_branches
)
- default_fix_versions = ",".join(default_fix_versions)
-
- issue_fix_versions = cmd.prompt("Enter comma-separated "
- "fix version(s) [%s]: "
- % default_fix_versions)
- if issue_fix_versions == "":
- issue_fix_versions = default_fix_versions
- issue_fix_versions = issue_fix_versions.replace(" ", "").split(",")
-
- def get_version_json(version_str):
- return [x for x in all_versions if x.name == version_str][0].raw
-
- return [get_version_json(v) for v in issue_fix_versions]
+ issue_fix_version = cmd.prompt("Enter comma-separated "
Review Comment:
Do we still support comma-separated fix version?
##########
dev/merge_arrow_pr.py:
##########
@@ -366,24 +491,25 @@ def maintenance_branches(self):
return [x["name"] for x in self._github_api.get_branches()
if x["name"].startswith("maint-")]
- def _get_jira(self):
+ def _get_issue(self):
if self.title.startswith("MINOR:"):
return None
- jira_id = None
- for project, regex in PR_TITLE_REGEXEN:
+ m = self.GITHUB_PR_TITLE_REGEXEN.search(self.title)
+ if m:
+ github_id = m.group(1)
+ return GitHubIssue(self._github_api, github_id, self.cmd)
+
+ for project, regex in self.JIRA_PR_TITLE_REGEXEN:
m = regex.search(self.title)
if m:
jira_id = m.group(1)
- break
-
- if jira_id is None:
- options = ' or '.join('{0}-XXX'.format(project)
- for project in SUPPORTED_PROJECTS)
- self.cmd.fail("PR title should be prefixed by a jira id "
- "{0}, but found {1}".format(options, self.title))
+ return JiraIssue(self.con, jira_id, project, self.cmd)
- return JiraIssue(self.con, jira_id, project, self.cmd)
+ options = ' or '.join('{0}-XXX'.format(project)
+ for project in self.JIRA_SUPPORTED_PROJECTS)
Review Comment:
Can we add `GH-XXX` too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
