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.

Sign up for GitHub

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

Jump to bottom

Fix update-release-branch-fix.py #566

Merged
merged 2 commits into from
Jun 17, 2021
Merged

Fix update-release-branch-fix.py #566

merged 2 commits into from
Jun 17, 2021

Conversation

aeisenberg
Copy link
Contributor

This change ensures that the the script can handle
commits with no committer in them. This will happen
for some commits that are auto-generated during
PRs.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

@aeisenberg
Copy link
Contributor Author

Darn...failing test-packaging-javascript workflow is unrelated, but my fault. I removed the special release of the CLI that this workflow requires. I'll fix.

adityasharad
adityasharad reviewed Jun 16, 2021
View reviewed changes
Copy link
Contributor

@adityasharad adityasharad left a comment

Choose a reason for hiding this comment

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

How come there was no committer to begin with? Did we use the wrong email for the bot?

.github/update-release-branch.py Outdated
@@ -122,7 +124,7 @@ def get_commit_difference(repo):

# Is the given commit the automatic merge commit from when merging a PR
def is_pr_merge_commit(commit):
return commit.committer.login == 'web-flow' and len(commit.parents) > 1
return commit.committer != None and commit.committer.login == 'web-flow' and len(commit.parents) > 1
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer this over equality checks.

Suggested change
return commit.committer != None and commit.committer.login == 'web-flow' and len(commit.parents) > 1
return commit.committer is not None and commit.committer.login == 'web-flow' and len(commit.parents) > 1

.github/update-release-branch.py Outdated
@@ -8,6 +8,8 @@
import datetime
import os

import inspect
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to remove that. Left over from debugging.

@aeisenberg
Fix update-release-branch-fix.py
d16268b 
This change ensures that the the script can handle
commits with no committer in them. This will happen
for some commits that are auto-generated during
PRs.
@aeisenberg aeisenberg force-pushed the aeisenberg/update-release-branch-fix branch from 84dc498 to df10b81 Compare June 16, 2021 17:39
@aeisenberg
Copy link
Contributor Author

OK...Still failing, even with the new CLI. The reason is different this time. With this new CLI, query and pack resolution is happening more correctly and the pack is failing to find it's library dependency codeql-javascript. In the previous version, the library path lookup was more lenient and was able to resolve the qlpack reference to the well-known codeql-javascript pack. So, I need to figure this out. I'll keep this PR open for now.

@aeisenberg
Fix custom version of CLI to use for package testing
efea923 
This can be removed when 2.5.6 is released.
@aeisenberg aeisenberg force-pushed the aeisenberg/update-release-branch-fix branch from df10b81 to efea923 Compare June 16, 2021 21:57
@aeisenberg
Copy link
Contributor Author

OK. This is now fixed. I had to recreate the codeql-pack1 and codeql-pack2 packs. Before, they were not properly created with their compile-time dependencies. Now they are.

@aeisenberg
Copy link
Contributor Author

Still failing on this branch when I run the Update release branch workflow. I'll have to inspect more.

@aeisenberg
Copy link
Contributor Author

Working now.

@aeisenberg aeisenberg merged commit 0b1f4a0 into main Jun 17, 2021
@aeisenberg aeisenberg deleted the aeisenberg/update-release-branch-fix branch June 17, 2021 17:22
@github-actions github-actions bot mentioned this pull request Jun 17, 2021
Merged
5 tasks
@aliscco aliscco mentioned this pull request Apr 24, 2023
Open
@snyk-bot snyk-bot mentioned this pull request Apr 24, 2023
Open
This was referenced Apr 24, 2023
Open
Open
Open
This was referenced Apr 24, 2023
Open
Open
Open
Open
This was referenced Apr 24, 2023
Open
Open
This was referenced Apr 24, 2023
Open
Open
Open
This was referenced Apr 30, 2023
Open
Open
Open
@chrisjcox79 chrisjcox79 mentioned this pull request Jun 20, 2023
Open
This was referenced Jun 21, 2023
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adityasharad adityasharad adityasharad approved these changes

@robertbrignull robertbrignull Awaiting requested review from robertbrignull

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aeisenberg @adityasharad