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

Use Issues/Timelines API to retrieve all previously requested reviewers #4

Merged
merged 51 commits into from
Feb 28, 2024

Conversation

jamoor-moj
Copy link

@jamoor-moj jamoor-moj commented Feb 27, 2024

The previous PR (#3) attempted to skip re-assigning aliases that had previously been assigned to the pull request.

Based on the github documentation, the previous API will only return current aliases that have been requested for review. This breaks two scenarios:

  1. user aliases that have previously signed off will not be returned (and therefore re-added)
  2. team aliases that have been signed off "on behalf of" a user alias will not be returned (they get removed entirely from the PR) (and therefore re-added).

This PR moves to using the timelineevents API which will give us every alias that has been requested regardless of their current status. Because the timeline can have a lot of events and the REST version doesn't allow us to filter, I opted to both swap to using GraphQL version and added pagination support..

The final change is to Unit Tests where we pull in rewire in order to mock private methods of our github.js file. Primarily due to issues with mocking a constructor now that we had to change the instantiation of octokit.

@jamoor-moj jamoor-moj marked this pull request as ready for review February 27, 2024 20:30
Copy link

@Warwolt Warwolt left a comment

Choose a reason for hiding this comment

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

I have some minor thoughts but otherwise I think this looks good to me (but I'm also not a GHA hacker so, would do good for others to give reviews as well)

src/index.js Outdated Show resolved Hide resolved
test/github.test.js Outdated Show resolved Hide resolved
src/github.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
@jamoor-moj jamoor-moj merged commit dc3795b into master Feb 28, 2024
1 check passed
@jamoor-moj jamoor-moj deleted the jamoor/timeline branch March 7, 2024 23:36
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.

3 participants