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

fix: using compareCommits for push event commit query #801

Merged
merged 3 commits into from
Sep 4, 2024

Conversation

Brian-Triplett
Copy link
Contributor

To actually resolve #752.

This logic uses the before and after shas in the push event with the compareCommits REST resource in OctoKit to get a list of commits between two shas. This should accomplish getting the commits included from an incoming push to a given ref

@Brian-Triplett
Copy link
Contributor Author

@wagoid I tested this locally with something like the following and it all seemed to work out. Let me know what you think!

const github = require('@actions/github')

async function run() {
  const octokit = github.getOctokit(process.env.GITHUB_TOKEN)

  // You can also pass in additional options as a second parameter to getOctokit
  // const octokit = github.getOctokit(myToken, {userAgent: "MyActionVersion1"});

  const { data: compareCommits } = await octokit.rest.repos.compareCommits({
    owner: 'wagoid',
    repo: 'commitlint-github-action',
    base: 'baa1b236f990293a1b2d94c19e41c2313a85e749',
    head: '0de15449e923d75a908bd7ab4e89c4e69af9ab1f',
    per_page: 100,
  })

  console.log(compareCommits.commits)
}

run()

@Brian-Triplett
Copy link
Contributor Author

hey @wagoid wanted to know if you had any questions or issues with this PR. This would help us out a lot and I'd rather not use our fork of this action if possible.

@wagoid
Copy link
Owner

wagoid commented Sep 3, 2024

Thanks @Brian-Triplett! Sorry for the delay, I could only have a look at this now.
Could you add a logic to handle times when the before is 0000000000000000000000000000000000000000? This happens in the first push, that's why we had a failure in the job of your PR. I've added a log here to demonstrate this happening:

before: 0000000000000000000000000000000000000000, after: 6d40f76ad6ef9b46be43ce925b1a1bc34c703b9d

In this case, I believe we should fallback to the commits coming from eventContext like it was before 💭

@wagoid
Copy link
Owner

wagoid commented Sep 3, 2024

In this PR you can better see this issue happening only in the first push: #803

@Brian-Triplett
Copy link
Contributor Author

@wagoid just pushed up the change to handle the first push scenario. Let me know what you think!

Copy link
Owner

@wagoid wagoid left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

@wagoid wagoid merged commit 47ff131 into wagoid:master Sep 4, 2024
2 checks passed
@Brian-Triplett Brian-Triplett deleted the fix/compare-commits-on-push branch September 4, 2024 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature request: commit list rest endpoint
2 participants