Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tools] handle commits merged without PR in deprecated script #10723

Merged
merged 6 commits into from
Apr 23, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 32 additions & 15 deletions tools/deprecate_version/deprecate_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,10 @@ def CreateIssues(access_token, runtime_and_pr):

Args:
access_token: GitHub access token (see comment at top of file).
runtime_and_pr: a list of runtime guards and the PRs they were added.
runtime_and_pr: a list of runtime guards and the PRs and commits they were added.
"""
repo = github.Github(access_token).get_repo('envoyproxy/envoy')
git = github.Github(access_token)
repo = git.get_repo('envoyproxy/envoy')

# Find GitHub label objects for LABELS.
labels = []
Expand All @@ -90,47 +91,62 @@ def CreateIssues(access_token, runtime_and_pr):
raise DeprecateVersionError('Unknown labels (expected %s, got %s)' % (LABELS, labels))

issues = []
for runtime_guard, pr in runtime_and_pr:
for runtime_guard, pr, commit in runtime_and_pr:
# Who is the author?
pr_info = repo.get_pull(pr)
if pr:
# Extract PR title, number, and author.
pr_info = repo.get_pull(pr)
change_title = pr_info.title
number = ('#%d') % pr
login = pr_info.user.login
else:
# Extract commit message, sha, and author.
# In this case we cannot add a user login for an assignee.
change_title = commit.message.split('\n')[0] # Remove description.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we constrain the length of this message a little bit beyond the newline char? I feel like people could be more verbose here than in a PR title.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh maybe, it looks like there's no enforced limit on the commit title. People seem to recommend 50/72 stylistically? I arbitrarily put 50.

number = ('commit %s') % commit.hexsha
email = commit.author.email
search_user = git.search_users(email.split('@')[0] + " in:email")
login = search_user[0].login if search_user else None

title = '%s deprecation' % (runtime_guard)
body = ('#%d (%s) introduced a runtime guarded feature. This issue '
'tracks source code cleanup.') % (pr, pr_info.title)
body = ('%s (%s) introduced a runtime guarded feature. This issue '
'tracks source code cleanup.') % (number, change_title)

print(title)
print(body)
print(' >> Assigning to %s' % pr_info.user.login)
print(' >> Assigning to %s' % (login if login else email))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can used shorthand ternary here:

Suggested change
print(' >> Assigning to %s' % (login if login else email))
print(' >> Assigning to %s' % (login or email))

Not sure which one is preferred by python people these days


# TODO(htuch): Figure out how to do this without legacy and faster.
exists = repo.legacy_search_issues('open', '"%s"' % title) or repo.legacy_search_issues(
'closed', '"%s"' % title)
if exists:
print(' >> Issue already exists, not posting!')
else:
issues.append((title, body, pr_info.user))
issues.append((title, body, login))

if not issues:
print('No features to deprecate in this release')
return

if GetConfirmation():
print('Creating issues...')
for title, body, user in issues:
for title, body, login in issues:
try:
repo.create_issue(title, body=body, assignees=[user.login], labels=labels)
repo.create_issue(title, body=body, assignees=[login], labels=labels)
except github.GithubException as e:
try:
body += '\ncc @' + user.login
if login:
body += '\ncc @' + login
repo.create_issue(title, body=body, labels=labels)
print(('unable to assign issue %s to %s. Add them to the Envoy proxy org'
'and assign it their way.') % (title, user.login))
'and assign it their way.') % (title, login))
except github.GithubException as e:
print('GithubException while creating issue.')
raise


def GetRuntimeAndPr():
"""Returns a list of tuples of [runtime features to deprecate, PR the feature was added]
"""Returns a list of tuples of [runtime features to deprecate, PR, commit the feature was added]
"""
repo = Repo(os.getcwd())

Expand Down Expand Up @@ -160,14 +176,15 @@ def GetRuntimeAndPr():
if runtime_guard == 'envoy.reloadable_features.test_feature_true':
found_test_feature_true = True
continue
pr = (int(re.search('\(#(\d+)\)', commit.message).group(1)))
pr_num = re.search('\(#(\d+)\)', commit.message)
pr = (int(pr_num.group(1))) if pr_num else None
asraa marked this conversation as resolved.
Show resolved Hide resolved
pr_date = date.fromtimestamp(commit.committed_date)
removable = (pr_date < removal_date)
# Add the runtime guard and PR to the list to file issues about.
print('Flag ' + runtime_guard + ' added at ' + str(pr_date) + ' ' +
(removable and 'and is safe to remove' or 'is not ready to remove'))
if removable:
features_to_flip.append((runtime_guard, pr))
features_to_flip.append((runtime_guard, pr, commit))
print('Failed to find test_feature_false. Script needs fixing')
sys.exit(1)

Expand Down
2 changes: 1 addition & 1 deletion tools/deprecate_version/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
GitPython==3.0.0
GitPython==3.1.0
PyGithub==1.43.8