Do not ignore staged PRs #66
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Initial (untested) implementation.
Tested and fixed several errors.
Such PR may be ready for merge and do not expect any activity/events before it gets merged.
Added logging and toString() conversions for PR numbers.
The previous approach could often result in situations when the optimization is discarded. It assumed that in most cases PR head SHA would stay the same across two PR scans (usually several minutes), but we cannot rely on that. Now we use PR branches instead for events<->PRs mapping.
It proved to be that these events have an empty 'pull_requests'
array for staged commits. To overcome this, we compile an array
of {sha, prNum} pairs for the staging branch and map an event's
sha to the PR number.
The previous approach was incorrect because the staged branch does not contain all the required commits for mapping to PR numbers (in fact, it has only one, head commit that is useful). There is no other way to approach this than to request the commit info from GitHub (entailing one extra API call). But, fortunately, the situation when we cannot extract this info from the current PR should be quite rare (i.e, the situation where there have been more than one staging branch commits during the previous PR scan).
We cannot rely on the 40-character check because branches can be easily a string of that length, but in a format allowed by GitHub (i.e., not in the '40 characters containing only 0-9 and A-F' format). The new PrId class should provide the required functionality to distinguish three PR ID types: SHAs, branches and PR numbers.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The recently added 265a79a optimization relied on PR::updated_at field
to track all PR-related changes. However, this proved to be wrong: for
example, PR or staged commit statuses do not update this field. Now we
get the missing information from incoming GitHub events.
Also do not ignore 'clearedForMerge' PRs: such PRs may be ready for
merge without additional GitHub activity.