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

Add Reverse Commits Option #288

Closed
wants to merge 1 commit into from
Closed

Conversation

ayeung-godaddy
Copy link

Based on Version Check Not Detecting Change Past 20 Commits #287
#287 (comment)

@@ -133,6 +133,12 @@ async function main() {
const commits =
eventObj.commits ||
(await request(eventObj.pull_request._links.commits.href))
console.log('DEBUG: original commits', commits)
if (getBooleanInput('reverse')) commits.reverse()
Copy link
Owner

Choose a reason for hiding this comment

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

Are you sure this will be enough?
If I remember correctly, the problem was that the API was not returning more than 20 commits from that link: so if that's the case, this would reverse the 20 oldest commits, not return the 20 newest :/

I think the proper solution would switch to using the Octokit to make a proper API call to the REST GitHub API to get the actual list of commits. This way we should have all the commits, and there should be no need to reverse them.
Or, even better, we can process the diff for the whole commit range in diff-search, so that we're not checking each commit's diff, but only one.

Copy link

github-actions bot commented Jun 6, 2024

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the status: stale Inactive issues and PRs label Jun 6, 2024
@github-actions github-actions bot closed this Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: stale Inactive issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants