kou commented on code in PR #33615:
URL: https://github.com/apache/arrow/pull/33615#discussion_r1070504373
##########
dev/archery/archery/release/core.py:
##########
@@ -285,8 +354,8 @@ def __repr__(self):
return f"<{self.__class__.__name__} {self.version!r} {status}>"
@staticmethod
- def from_jira(version, jira=None, repo=None):
- return Release(version, jira, repo)
+ def from_issue_tracker(version, issue_tracker=None, repo=None):
Review Comment:
Can we remove this? It seems that this is used only for test.
##########
dev/archery/archery/release/core.py:
##########
@@ -250,15 +323,9 @@ def __new__(self, version, jira=None, repo=None):
return super().__new__(klass)
- def __init__(self, version, jira, repo):
- if jira is None:
- jira = Jira()
- elif isinstance(jira, str):
- jira = Jira(jira)
- elif not isinstance(jira, (Jira, CachedJira)):
- raise TypeError("`jira` argument must be a server url or a valid "
- "Jira instance")
-
+ def __init__(self, version, repo, github_token=None, issue_tracker=None):
+ if not issue_tracker:
+ issue_tracker = IssueTracker(github_token=github_token)
Review Comment:
How about requiring `issue_tracker` instead of accepting optional
`github_token`?
##########
dev/archery/archery/release/core.py:
##########
@@ -74,68 +90,122 @@ def from_jira(cls, jira_issue):
summary=jira_issue.fields.summary
)
+ @classmethod
+ def from_github(cls, github_issue):
+ original_jira = cls.original_jira_id(github_issue)
+ key = original_jira or github_issue.number
+ return cls(
+ key=key,
+ type=next(
+ iter(
+ [
+ label.name for label in github_issue.labels
+ if label.name.startswith("Type:")
+ ]
+ ), None),
+ summary=github_issue.title,
+ github_issue=github_issue
+ )
+
@property
def project(self):
+ if isinstance(self.key, int):
+ return 'GH'
return self.key.split('-')[0]
@property
def number(self):
- return int(self.key.split('-')[1])
+ if isinstance(self.key, str):
+ return int(self.key.split('-')[1])
+ else:
+ return self.key
+
+ @cached_property
+ def pr(self):
+ if self.is_pr:
+ return self._github_issue.as_pull_request()
+
+ @cached_property
+ def is_pr(self):
+ return bool(self._github_issue and self._github_issue.pull_request)
+
+ @property
+ def components(self):
+ if self._github_issue:
+ return [label for label in self._github_issue.labels
+ if label.name.startswith("Component:")]
+
+ def is_component(self, component_name):
+ if components := self.components:
+ for component in components:
+ if component.name == f"Component: {component_name}":
+ return True
+ return False
Review Comment:
It seems that they aren't used.
##########
dev/archery/archery/release/core.py:
##########
@@ -74,68 +90,122 @@ def from_jira(cls, jira_issue):
summary=jira_issue.fields.summary
)
+ @classmethod
+ def from_github(cls, github_issue):
+ original_jira = cls.original_jira_id(github_issue)
+ key = original_jira or github_issue.number
+ return cls(
+ key=key,
+ type=next(
+ iter(
+ [
+ label.name for label in github_issue.labels
+ if label.name.startswith("Type:")
+ ]
+ ), None),
+ summary=github_issue.title,
+ github_issue=github_issue
+ )
+
@property
def project(self):
+ if isinstance(self.key, int):
+ return 'GH'
return self.key.split('-')[0]
@property
def number(self):
- return int(self.key.split('-')[1])
+ if isinstance(self.key, str):
+ return int(self.key.split('-')[1])
+ else:
+ return self.key
+
+ @cached_property
+ def pr(self):
+ if self.is_pr:
+ return self._github_issue.as_pull_request()
+
+ @cached_property
+ def is_pr(self):
+ return bool(self._github_issue and self._github_issue.pull_request)
Review Comment:
It seems that `self._github_issue` is needed only for this. How about
setting `self.is_pr` instead of keeping `self._github_issue`?
##########
dev/archery/archery/release/core.py:
##########
@@ -74,68 +90,122 @@ def from_jira(cls, jira_issue):
summary=jira_issue.fields.summary
)
+ @classmethod
+ def from_github(cls, github_issue):
+ original_jira = cls.original_jira_id(github_issue)
+ key = original_jira or github_issue.number
+ return cls(
+ key=key,
+ type=next(
+ iter(
+ [
+ label.name for label in github_issue.labels
+ if label.name.startswith("Type:")
+ ]
+ ), None),
+ summary=github_issue.title,
+ github_issue=github_issue
+ )
+
@property
def project(self):
+ if isinstance(self.key, int):
+ return 'GH'
return self.key.split('-')[0]
@property
def number(self):
- return int(self.key.split('-')[1])
+ if isinstance(self.key, str):
+ return int(self.key.split('-')[1])
+ else:
+ return self.key
+
+ @cached_property
+ def pr(self):
+ if self.is_pr:
+ return self._github_issue.as_pull_request()
Review Comment:
It seems that this is not used.
##########
dev/archery/archery/release/core.py:
##########
@@ -269,13 +336,15 @@ def __init__(self, version, jira, repo):
"instance")
if isinstance(version, str):
- version = jira.project_version(version, project='ARROW')
+ version = issue_tracker.project_version(version)
+
elif not isinstance(version, Version):
raise TypeError(version)
self.version = version
- self.jira = jira
self.repo = repo
+ self.issue_tracker = issue_tracker
+ self._github_token = github_token
Review Comment:
It seems that this is needless.
--
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]
