Skip to content

Commit

Permalink
Fix some review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
raulcd committed Jan 16, 2023
1 parent 70f6740 commit 462287a
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 59 deletions.
21 changes: 11 additions & 10 deletions dev/archery/archery/release/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
def release(obj, src, github_token):
"""Release releated commands."""

obj['github_token'] = github_token
obj['issue_tracker'] = IssueTracker(github_token=github_token)
obj['repo'] = src.path


Expand All @@ -46,7 +46,7 @@ def release(obj, src, github_token):
def release_curate(obj, version, minimal):
"""Release curation."""
release = Release(version, repo=obj['repo'],
github_token=obj['github_token'])
issue_tracker=obj['issue_tracker'])
curation = release.curate(minimal)

click.echo(curation.render('console'))
Expand All @@ -63,10 +63,10 @@ def release_changelog():
@click.pass_obj
def release_changelog_add(obj, version):
"""Prepend the changelog with the current release"""
repo, github_token = obj['repo'], obj['github_token']
repo, issue_tracker = obj['repo'], obj['issue_tracker']

# just handle the current version
release = Release(version, repo=repo, github_token=github_token)
release = Release(version, repo=repo, issue_tracker=issue_tracker)
if release.is_released:
raise ValueError('This version has been already released!')

Expand All @@ -86,10 +86,10 @@ def release_changelog_add(obj, version):
@click.pass_obj
def release_changelog_generate(obj, version, output):
"""Generate the changelog of a specific release."""
repo, github_token = obj['repo'], obj['github_token']
repo, issue_tracker = obj['repo'], obj['issue_tracker']

# just handle the current version
release = Release(version, repo=repo, github_token=github_token)
release = Release(version, repo=repo, issue_tracker=issue_tracker)

changelog = release.changelog()
output.write(changelog.render('markdown'))
Expand All @@ -99,15 +99,15 @@ def release_changelog_generate(obj, version, output):
@click.pass_obj
def release_changelog_regenerate(obj):
"""Regeneretate the whole CHANGELOG.md file"""
github_token, repo = obj['github_token'], obj['repo']
issue_tracker, repo = obj['issue_tracker'], obj['repo']
changelogs = []
issue_tracker = IssueTracker(github_token=github_token)
issue_tracker = IssueTracker(issue_tracker=issue_tracker)

for version in issue_tracker.project_versions():
if not version.released:
continue
release = Release(version, repo=repo,
github_token=github_token)
issue_tracker=issue_tracker)
click.echo('Querying changelog for version: {}'.format(version))
changelogs.append(release.changelog())

Expand All @@ -130,8 +130,9 @@ def release_cherry_pick(obj, version, dry_run, recreate):
"""
Cherry pick commits.
"""
issue_tracker = obj['issue_tracker']
release = Release(version,
repo=obj['repo'], github_token=obj['github_token'])
repo=obj['repo'], issue_tracker=issue_tracker)

if not dry_run:
release.cherry_pick_commits(recreate_branch=recreate)
Expand Down
37 changes: 5 additions & 32 deletions dev/archery/archery/release/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,28 +120,10 @@ def number(self):
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

@classmethod
def original_jira_id(cls, github_issue):
# All migrated issues contain body
Expand Down Expand Up @@ -323,9 +305,7 @@ def __new__(self, version, repo=None, github_token=None,

return super().__new__(klass)

def __init__(self, version, repo, github_token=None, issue_tracker=None):
if not issue_tracker:
issue_tracker = IssueTracker(github_token=github_token)
def __init__(self, version, repo, issue_tracker):
if repo is None:
arrow = ArrowSources.find()
repo = Repo(arrow.path)
Expand All @@ -344,7 +324,6 @@ def __init__(self, version, repo, github_token=None, issue_tracker=None):
self.version = version
self.repo = repo
self.issue_tracker = issue_tracker
self._github_token = github_token

def __repr__(self):
if self.version.released:
Expand All @@ -353,10 +332,6 @@ def __repr__(self):
status = "pending"
return f"<{self.__class__.__name__} {self.version!r} {status}>"

@staticmethod
def from_issue_tracker(version, issue_tracker=None, repo=None):
return Release(version, repo, issue_tracker=issue_tracker)

@property
def is_released(self):
return self.version.released
Expand Down Expand Up @@ -392,8 +367,7 @@ def previous(self):
return None
else:
return Release(previous, repo=self.repo,
issue_tracker=self.issue_tracker,
github_token=self._github_token)
issue_tracker=self.issue_tracker)

@cached_property
def next(self):
Expand All @@ -404,8 +378,7 @@ def next(self):
f"version {self.version}")
upcoming = self.siblings[position - 1]
return Release(upcoming, repo=self.repo,
issue_tracker=self.issue_tracker,
github_token=self._github_token)
issue_tracker=self.issue_tracker)

@cached_property
def issues(self):
Expand Down Expand Up @@ -564,8 +537,8 @@ def changelog(self):
try:
categories[issue_types[issue.type]].append((issue, commit))
except KeyError:
# If issue is a PR and don't have a type assume task
assert issue.is_pr
# If issue or pr don't have a type assume task.
# Currently the label for type is not mandatory on GitHub.
categories[issue_types['Type: task']].append((issue, commit))

# sort issues by the issue key in ascending order
Expand Down
34 changes: 17 additions & 17 deletions dev/archery/archery/release/tests/test_release.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,68 +230,68 @@ def test_commit_title():


def test_release_basics(fake_issue_tracker):
r = Release.from_issue_tracker("1.0.0", issue_tracker=fake_issue_tracker)
r = Release("1.0.0", repo=None, issue_tracker=fake_issue_tracker)
assert isinstance(r, MajorRelease)
assert r.is_released is True
assert r.branch == 'maint-1.0.0'
assert r.tag == 'apache-arrow-1.0.0'

r = Release.from_issue_tracker("1.1.0", issue_tracker=fake_issue_tracker)
r = Release("1.1.0", repo=None, issue_tracker=fake_issue_tracker)
assert isinstance(r, MinorRelease)
assert r.is_released is False
assert r.branch == 'maint-1.x.x'
assert r.tag == 'apache-arrow-1.1.0'

# minor releases before 1.0 are treated as major releases
r = Release.from_issue_tracker("0.17.0", issue_tracker=fake_issue_tracker)
r = Release("0.17.0", repo=None, issue_tracker=fake_issue_tracker)
assert isinstance(r, MajorRelease)
assert r.is_released is True
assert r.branch == 'maint-0.17.0'
assert r.tag == 'apache-arrow-0.17.0'

r = Release.from_issue_tracker("0.17.1", issue_tracker=fake_issue_tracker)
r = Release("0.17.1", repo=None, issue_tracker=fake_issue_tracker)
assert isinstance(r, PatchRelease)
assert r.is_released is True
assert r.branch == 'maint-0.17.x'
assert r.tag == 'apache-arrow-0.17.1'


def test_previous_and_next_release(fake_issue_tracker):
r = Release.from_issue_tracker("4.0.0", issue_tracker=fake_issue_tracker)
r = Release("4.0.0", repo=None, issue_tracker=fake_issue_tracker)
assert isinstance(r.previous, MajorRelease)
assert r.previous.version == Version.parse("3.0.0")
with pytest.raises(ValueError, match="There is no upcoming release set"):
assert r.next

r = Release.from_issue_tracker("3.0.0", issue_tracker=fake_issue_tracker)
r = Release("3.0.0", repo=None, issue_tracker=fake_issue_tracker)
assert isinstance(r.previous, MajorRelease)
assert isinstance(r.next, MajorRelease)
assert r.previous.version == Version.parse("2.0.0")
assert r.next.version == Version.parse("4.0.0")

r = Release.from_issue_tracker("1.1.0", issue_tracker=fake_issue_tracker)
r = Release("1.1.0", repo=None, issue_tracker=fake_issue_tracker)
assert isinstance(r.previous, MajorRelease)
assert isinstance(r.next, MajorRelease)
assert r.previous.version == Version.parse("1.0.0")
assert r.next.version == Version.parse("2.0.0")

r = Release.from_issue_tracker("1.0.0", issue_tracker=fake_issue_tracker)
r = Release("1.0.0", repo=None, issue_tracker=fake_issue_tracker)
assert isinstance(r.next, MajorRelease)
assert isinstance(r.previous, MajorRelease)
assert r.previous.version == Version.parse("0.17.0")
assert r.next.version == Version.parse("2.0.0")

r = Release.from_issue_tracker("0.17.0", issue_tracker=fake_issue_tracker)
r = Release("0.17.0", repo=None, issue_tracker=fake_issue_tracker)
assert isinstance(r.previous, MajorRelease)
assert r.previous.version == Version.parse("0.16.0")

r = Release.from_issue_tracker("0.15.2", issue_tracker=fake_issue_tracker)
r = Release("0.15.2", repo=None, issue_tracker=fake_issue_tracker)
assert isinstance(r.previous, PatchRelease)
assert isinstance(r.next, MajorRelease)
assert r.previous.version == Version.parse("0.15.1")
assert r.next.version == Version.parse("0.16.0")

r = Release.from_issue_tracker("0.15.1", issue_tracker=fake_issue_tracker)
r = Release("0.15.1", repo=None, issue_tracker=fake_issue_tracker)
assert isinstance(r.previous, MajorRelease)
assert isinstance(r.next, PatchRelease)
assert r.previous.version == Version.parse("0.15.0")
Expand All @@ -300,7 +300,7 @@ def test_previous_and_next_release(fake_issue_tracker):

def test_release_issues(fake_issue_tracker):
# major release issues
r = Release.from_issue_tracker("1.0.0", issue_tracker=fake_issue_tracker)
r = Release("1.0.0", repo=None, issue_tracker=fake_issue_tracker)
assert r.issues.keys() == set([
"ARROW-300",
"ARROW-4427",
Expand All @@ -312,7 +312,7 @@ def test_release_issues(fake_issue_tracker):
"ARROW-8973"
])
# minor release issues
r = Release.from_issue_tracker("0.17.0", issue_tracker=fake_issue_tracker)
r = Release("0.17.0", repo=None, issue_tracker=fake_issue_tracker)
assert r.issues.keys() == set([
"ARROW-2882",
"ARROW-2587",
Expand All @@ -322,7 +322,7 @@ def test_release_issues(fake_issue_tracker):
"ARROW-1636",
])
# patch release issues
r = Release.from_issue_tracker("1.0.1", issue_tracker=fake_issue_tracker)
r = Release("1.0.1", repo=None, issue_tracker=fake_issue_tracker)
assert r.issues.keys() == set([
"ARROW-9684",
"ARROW-9667",
Expand All @@ -332,7 +332,7 @@ def test_release_issues(fake_issue_tracker):
"ARROW-9609",
"ARROW-9606"
])
r = Release.from_issue_tracker("2.0.0", issue_tracker=fake_issue_tracker)
r = Release("2.0.0", repo=None, issue_tracker=fake_issue_tracker)
assert r.issues.keys() == set([
"ARROW-9784",
"ARROW-9767",
Expand All @@ -351,7 +351,7 @@ def test_release_issues(fake_issue_tracker):
("0.15.1", 41)
])
def test_release_commits(fake_issue_tracker, version, ncommits):
r = Release.from_issue_tracker(version, issue_tracker=fake_issue_tracker)
r = Release(version, repo=None, issue_tracker=fake_issue_tracker)
assert len(r.commits) == ncommits
for c in r.commits:
assert isinstance(c, Commit)
Expand All @@ -360,7 +360,7 @@ def test_release_commits(fake_issue_tracker, version, ncommits):


def test_maintenance_patch_selection(fake_issue_tracker):
r = Release.from_issue_tracker("0.17.1", issue_tracker=fake_issue_tracker)
r = Release("0.17.1", repo=None, issue_tracker=fake_issue_tracker)

shas_to_pick = [
c.hexsha for c in r.commits_to_pick(exclude_already_applied=False)
Expand Down

0 comments on commit 462287a

Please sign in to comment.