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

feat(reviewersInTeams): using teams to add reviews #125

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

itomtom
Copy link

@itomtom itomtom commented Apr 14, 2020

  • Using GitHub teams you can specify reviewers to be added to Pull
    Request
  • GitHub team's slug name is used under the label reviewersInTeams to
    extract the members in the team

Fixes #123

@kentaro-m kentaro-m self-requested a review April 14, 2020 13:06
@kentaro-m
Copy link
Owner

kentaro-m commented Apr 18, 2020

Hi, @itomtom! Thanks for adding a new feature. I have reviewed it.

This change needs the scope of members to call GitHub Teams API.
c.f. https://developer.github.com/v3/apps/permissions/#permission-on-members

https://github.com/kentaro-m/auto-assign/blob/master/app.yml#L105

members: read

Also, I have checked your changes in my local environment and failed unit tests. I think I need to update the new version of the Probot.

スクリーンショット 2020-04-18 20 24 47

probot/probot#1128

$ cd auto-assign
$ rm -rf node_modules
$ npm install
$ npm update probot --save
diff --git a/package.json b/package.json
index 5cfa772..dd01781 100644
--- a/package.json
+++ b/package.json
@@ -39,7 +39,7 @@
     "@types/lodash": "^4.14.119",
     "@types/node": "^10.12.18",
     "lodash": "^4.17.15",
-    "probot": "^9.6.3",
+    "probot": "^9.11.3",
     "typescript": "^3.7.2"
   },
   "devDependencies": {

I have not done a detailed review yet, so I will continue to check it.

* Using GitHub teams you can specify reviewers to be added to Pull
Request
* GitHub team's slug name is used under the label reviewersInTeams to
extract the members in the team

Fixes kentaro-m#123
@itomtom itomtom force-pushed the ttruong/feat/reviewersInTeams branch from a63a68d to d7e792b Compare April 21, 2020 01:30
@gmazzo
Copy link

gmazzo commented May 14, 2020

Is this PR stale?

@itomtom
Copy link
Author

itomtom commented Aug 13, 2020

I'm just waiting for further reviews as I made an update push on 21st April.

@kentaro-m
Copy link
Owner

Hi, @itomtom.

Building these codes and running unit tests are failed in my environment (Node.js v12.18.3).
How it behaves in your environment?

~/workspace/github.com/kentaro-m/auto-assign ttruong/feat/reviewersInTeams 19s
❯ npm run build

> auto-assign@1.0.0 build /Users/kentarom/workspace/github.com/kentaro-m/auto-assign
> tsc -p tsconfig.json

src/handler.ts:27:34 - error TS2339: Property 'listMembersInOrg' does not exist on type '{ addMember: { (params?: (RequestOptions & TeamsAddMemberParams) | undefined): Promise<Response<TeamsAddMemberResponse>>; endpoint: Endpoint; }; ... 32 more ...; updateDiscussionComment: { ...; }; }'.

27       await context.github.teams.listMembersInOrg({
                                    ~~~~~~~~~~~~~~~~

src/handler.ts:57:36 - error TS2339: Property 'listMembersLegacy' does not exist on type '{ addMember: { (params?: (RequestOptions & TeamsAddMemberParams) | undefined): Promise<Response<TeamsAddMemberResponse>>; endpoint: Endpoint; }; ... 32 more ...; updateDiscussionComment: { ...; }; }'.

57         await context.github.teams.listMembersLegacy({
                                      ~~~~~~~~~~~~~~~~~


Found 2 errors.

npm ERR! code ELIFECYCLE
npm ERR! errno 2
npm ERR! auto-assign@1.0.0 build: `tsc -p tsconfig.json`
npm ERR! Exit status 2
npm ERR!
npm ERR! Failed at the auto-assign@1.0.0 build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/kentarom/.npm/_logs/2020-08-14T16_09_27_437Z-debug.log

~/workspace/github.com/kentaro-m/auto-assign ttruong/feat/reviewersInTeams
❯ npm test

> auto-assign@1.0.0 test /Users/kentarom/workspace/github.com/kentaro-m/auto-assign
> jest

 PASS  test/util.test.ts
 FAIL  test/handler.test.ts
  ● Test suite failed to run

    TypeScript diagnostics (customize using `[jest-config].globals.ts-jest.diagnostics` option):
    test/handler.test.ts:943:50 - error TS2345: Argument of type '"listMembersLegacy"' is not assignable to parameter of type '"create" | "get" | "list" | "listForAuthenticatedUser" | "update" | "addMember" | "addOrUpdateMembership" | "addOrUpdateProject" | "addOrUpdateRepo" | "checkManagesRepo" | ... 23 more ... | "updateDiscussionComment"'.

    943     const spy = jest.spyOn(context.github.teams, 'listMembersLegacy')
                                                         ~~~~~~~~~~~~~~~~~~~

Test Suites: 1 failed, 1 passed, 2 total
Tests:       15 passed, 15 total
Snapshots:   0 total
Time:        2.387s, estimated 3s
Ran all test suites.
npm ERR! Test failed.  See above for more details.

const addAssigneesSpy = jest.spyOn(context.github.issues, 'addAssignees')
const createReviewRequestSpy = jest.spyOn(
const addAssigneesSpy: any = jest.spyOn(context.github.issues, 'addAssignees')
const createReviewRequestSpy: any = jest.spyOn(
Copy link

Choose a reason for hiding this comment

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

.github/auto-assign.yml

@@ -365,7 +365,7 @@ describe('handlePullRequest', () => {
})
} as any

const spy = jest.spyOn(context.github.issues, 'addAssignees')
const spy: any = jest.spyOn(context.github.issues, 'addAssignees')

await handlePullRequest(context)
Copy link

Choose a reason for hiding this comment

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

.github/auto-assign.yml

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.

Using github team to define the list of reviewers instead of listening individual names
4 participants