paritytech / pr-custom-review

GitHub Action for complex pull request approval cases that are not currently supported by the Branch protection feature in GitHub.
MIT License
8 stars 4 forks source link

Abstracted code logic #108

Closed Bullrich closed 1 year ago

Bullrich commented 1 year ago

Abstracted GitHub api interaction from the runner method.

This is the first step into isolating the code so it is easier to read.

Second step will isolate the logic

mutantcornholio commented 1 year ago

I feel like this duplicates the purpose of opstooling-integrations. Should we consider reusing it instead?

Bullrich commented 1 year ago

opstooling-integrations

I would be up to use it but would need to check if I can adapt the specific endpoints and combinations that are being called.

Some stuff, like the following, may not be that straightforward to migrate.

octokit.paginate(
      this.octokit.rest.teams.listMembersInOrg,
      { org:pr.base.repo.owner.login, team_slug, per_page: maxGithubApiTeamMembersPerPage },
      (response) => response.data.map(({ login }) => login),
    );

And I would still keep it in a wrapper so it can be tested easier.

But this PR would be a step forward into that direction

mutantcornholio commented 1 year ago

Some stuff, like the following, may not be that straightforward to migrate.

octokit.paginate(
      this.octokit.rest.teams.listMembersInOrg,
      { org:pr.base.repo.owner.login, team_slug, per_page: maxGithubApiTeamMembersPerPage },
      (response) => response.data.map(({ login }) => login),
    );

We have paginated methods there :)

And I would still keep it in a wrapper so it can be tested easier.

One of the major points of creating opstooling-integrations was this exact goal: providing mocks for the endpoints from ground up, ease of use with jest.mock.

But this PR would be a step forward into that direction

Cool