From cf69bc38c7b5756542607a8fb30aec03b4f5782d Mon Sep 17 00:00:00 2001 From: Adrien Clerbois Date: Wed, 13 Aug 2025 15:05:09 +0200 Subject: [PATCH 1/2] feat: add support for separate team reviewers in configuration --- CHANGEMENTS.md | 60 ++++++++++++++++++ README.md | 7 ++ auto_assign_example.yml | 42 ++++++++++++ src/handler.ts | 1 + src/util.ts | 24 +++++-- test/handler.test.ts | 72 ++++++++++++++++++++- test/util.test.ts | 137 +++++++++++++++++++++++++++++----------- 7 files changed, 301 insertions(+), 42 deletions(-) create mode 100644 CHANGEMENTS.md create mode 100644 auto_assign_example.yml diff --git a/CHANGEMENTS.md b/CHANGEMENTS.md new file mode 100644 index 0000000..b3df7cc --- /dev/null +++ b/CHANGEMENTS.md @@ -0,0 +1,60 @@ +# Changements apportés pour supporter les team_reviewers séparés + +## Résumé +Ajout de la possibilité de spécifier les `team_reviewers` dans un tableau séparé des `reviewers` individuels dans la configuration. + +## Modifications apportées + +### 1. Interface Config (src/handler.ts) +- Ajout de la propriété `team_reviewers: string[]` à l'interface `Config` + +### 2. Logique de sélection des reviewers (src/util.ts) +- Modification de la fonction `chooseReviewers()` pour traiter séparément : + - `reviewers` : tableau des utilisateurs individuels (filtré pour exclure le créateur de la PR) + - `team_reviewers` : tableau des équipes (utilisé tel quel) +- Le `numberOfReviewers` n'affecte que les reviewers individuels, pas les équipes + +### 3. Documentation (README.md) +- Mise à jour de la section "Single Reviewers List" pour montrer l'utilisation de `team_reviewers` +- Suppression de l'ancienne section qui expliquait l'utilisation de la syntaxe `/team` dans `reviewers` +- Clarification que `numberOfReviewers` n'affecte que les reviewers individuels + +### 4. Tests +- Ajout de tests pour vérifier le bon fonctionnement des `team_reviewers` séparés +- Mise à jour des tests existants pour utiliser la nouvelle structure +- Tests pour les cas de configurations vides ou manquantes + +### 5. Exemple de configuration +- Création d'un fichier `auto_assign_example.yml` montrant la nouvelle syntaxe + +## Usage + +### Avant (ancienne méthode - toujours supportée pour les groupes) +```yaml +reviewers: + - reviewer1 + - reviewer2 + - /team1 + - org/team2 +``` + +### Maintenant (nouvelle méthode recommandée) +```yaml +reviewers: + - reviewer1 + - reviewer2 + +team_reviewers: + - team1 + - org/team2 +``` + +## Avantages de cette approche +1. **Séparation claire** : Les utilisateurs et équipes sont clairement séparés +2. **Contrôle granulaire** : `numberOfReviewers` n'affecte que les utilisateurs individuels +3. **Facilité de configuration** : Plus facile de configurer et comprendre +4. **Compatibilité** : Les groupes continuent de fonctionner comme avant +5. **Pas de limite** : Toutes les équipes spécifiées sont ajoutées (pas de sélection aléatoire) + +## Rétro-compatibilité +Cette modification est rétro-compatible. Si `team_reviewers` n'est pas spécifié, il sera traité comme un tableau vide et les équipes peuvent toujours être spécifiées dans `reviewers` via les groupes de review. diff --git a/README.md b/README.md index 3efebd0..e922bd0 100644 --- a/README.md +++ b/README.md @@ -29,8 +29,15 @@ reviewers: - reviewerB - reviewerC +# A list of team reviewers to be added to pull requests (GitHub team slug) +team_reviewers: + - org/teamReviewerA + - org/teamReviewerB + - teamReviewerC + # A number of reviewers added to the pull request # Set 0 to add all the reviewers (default: 0) +# Note: This only affects individual reviewers, not team reviewers numberOfReviewers: 0 # A list of assignees, overrides reviewers if set diff --git a/auto_assign_example.yml b/auto_assign_example.yml new file mode 100644 index 0000000..652f686 --- /dev/null +++ b/auto_assign_example.yml @@ -0,0 +1,42 @@ +# Configuration example for auto-assign with separate team_reviewers + +# Set to true to add reviewers to pull requests +addReviewers: true + +# Set to true to add assignees to pull requests +addAssignees: false + +# A list of individual reviewers to be added to pull requests (GitHub user name) +reviewers: + - reviewerA + - reviewerB + - reviewerC + +# A list of team reviewers to be added to pull requests (GitHub team slug) +# Teams can be specified with or without the org prefix +team_reviewers: + - frontend-team + - backend-team + - org/security-team + +# A number of individual reviewers added to the pull request +# Set 0 to add all the reviewers (default: 0) +# Note: This only affects individual reviewers, not team reviewers +numberOfReviewers: 2 + +# A list of assignees, overrides reviewers if set +# assignees: +# - assigneeA + +# A number of assignees to add to the pull request +# Set to 0 to add all of the assignees. +# Uses numberOfReviewers if unset. +# numberOfAssignees: 2 + +# A list of keywords to be skipped the process that add reviewers if pull requests include it +# skipKeywords: +# - wip + +# A list of users to be skipped by both the add reviewers and add assignees processes +# skipUsers: +# - dependabot[bot] diff --git a/src/handler.ts b/src/handler.ts index 25c5299..894215d 100644 --- a/src/handler.ts +++ b/src/handler.ts @@ -9,6 +9,7 @@ export interface Config { addReviewers: boolean addAssignees: boolean reviewers: string[] + teamReviewers: string[] assignees: string[] numberOfAssignees: number numberOfReviewers: number diff --git a/src/util.ts b/src/util.ts index 7d9e38e..1a3e511 100644 --- a/src/util.ts +++ b/src/util.ts @@ -62,7 +62,13 @@ export function chooseReviewers( reviewers: string[] team_reviewers: string[] } { - const { useReviewGroups, reviewGroups, numberOfReviewers, reviewers } = config + const { + useReviewGroups, + reviewGroups, + numberOfReviewers, + reviewers, + teamReviewers: team_reviewers, + } = config const useGroups: boolean = useReviewGroups && Object.keys(reviewGroups).length > 0 @@ -79,11 +85,21 @@ export function chooseReviewers( } } - const chosenReviewers = chooseUsers(reviewers, numberOfReviewers, owner) + // Filter out the owner from individual reviewers + const filteredReviewers = reviewers + ? reviewers.filter((reviewer) => reviewer !== owner) + : [] + + // Choose individual reviewers + const chosenIndividualReviewers = + numberOfReviewers > 0 + ? _.sampleSize(filteredReviewers, numberOfReviewers) + : filteredReviewers + // Return both individual reviewers and team reviewers return { - reviewers: chosenReviewers.users, - team_reviewers: chosenReviewers.teams, + reviewers: chosenIndividualReviewers, + team_reviewers: team_reviewers || [], } } diff --git a/test/handler.test.ts b/test/handler.test.ts index b568ff6..f977f7e 100644 --- a/test/handler.test.ts +++ b/test/handler.test.ts @@ -142,7 +142,8 @@ describe('handlePullRequest', () => { addAssignees: false, addReviewers: true, numberOfReviewers: 0, - reviewers: ['/team_reviewer1'], + reviewers: [], + teamReviewers: ['team_reviewer1'], skipKeywords: ['wip'], } }) @@ -1008,4 +1009,73 @@ describe('handlePullRequest', () => { expect(requestReviewersSpy).not.toBeCalled() expect(addAssigneesSpy).not.toBeCalled() }) + + test('adds team reviewers from separate team_reviewers config', async () => { + // GIVEN + context.config = jest.fn().mockImplementation(async () => { + return { + addReviewers: true, + addAssignees: false, + reviewers: ['reviewer1', 'reviewer2'], + teamReviewers: ['team1', 'org/team2'], + numberOfReviewers: 0, + } + }) + + context.octokit.pulls = { + requestReviewers: jest.fn().mockImplementation(async () => ({})), + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any + + const requestReviewersSpy = jest.spyOn( + context.octokit.pulls, + 'requestReviewers' + ) + + // WHEN + await handlePullRequest(context) + + // THEN + expect(requestReviewersSpy).toBeCalledTimes(1) + expect(requestReviewersSpy.mock.calls[0][0]?.reviewers).toEqual([ + 'reviewer1', + 'reviewer2', + ]) + expect(requestReviewersSpy.mock.calls[0][0]?.team_reviewers).toEqual([ + 'team1', + 'org/team2', + ]) + }) + + test('handles empty team_reviewers config gracefully', async () => { + // GIVEN + context.config = jest.fn().mockImplementation(async () => { + return { + addReviewers: true, + addAssignees: false, + reviewers: ['reviewer1'], + numberOfReviewers: 0, + } + }) + + context.octokit.pulls = { + requestReviewers: jest.fn().mockImplementation(async () => ({})), + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any + + const requestReviewersSpy = jest.spyOn( + context.octokit.pulls, + 'requestReviewers' + ) + + // WHEN + await handlePullRequest(context) + + // THEN + expect(requestReviewersSpy).toBeCalledTimes(1) + expect(requestReviewersSpy.mock.calls[0][0]?.reviewers).toEqual([ + 'reviewer1', + ]) + expect(requestReviewersSpy.mock.calls[0][0]?.team_reviewers).toEqual([]) + }) }) diff --git a/test/util.test.ts b/test/util.test.ts index 20f09a7..3d646c5 100644 --- a/test/util.test.ts +++ b/test/util.test.ts @@ -1,4 +1,9 @@ -import { chooseUsers, chooseUsersFromGroups, includesSkipKeywords } from '../src/util' +import { + chooseUsers, + chooseUsersFromGroups, + chooseReviewers, + includesSkipKeywords, +} from '../src/util' describe('chooseUsers', () => { test('returns the reviewer list without the PR creator', () => { @@ -98,13 +103,8 @@ describe('chooseUsersFromGroups', () => { // GIVEN const owner = 'owner' const reviewers = { - groupA: [ - 'owner', - 'reviewer1' - ], - groupB: [ - 'reviewer2' - ] + groupA: ['owner', 'reviewer1'], + groupB: ['reviewer2'], } const numberOfReviewers = 1 @@ -119,12 +119,8 @@ describe('chooseUsersFromGroups', () => { // GIVEN const owner = 'owner' const reviewers = { - groupA: [ - 'owner' - ], - groupB: [ - 'reviewer2' - ] + groupA: ['owner'], + groupB: ['reviewer2'], } const numberOfReviewers = 1 @@ -140,20 +136,10 @@ describe('chooseUsersFromGroups', () => { // GIVEN const owner = 'owner' const reviewers = { - groupA: [ - 'owner', - 'groupA-1', - 'groupA-2' - ], - groupB: [ - 'groupB-1', - 'groupB-2' - ], + groupA: ['owner', 'groupA-1', 'groupA-2'], + groupB: ['groupB-1', 'groupB-2'], groupC: [], - groupD: [ - 'groupD-1', - 'groupD-2' - ] + groupD: ['groupD-1', 'groupD-2'], } const numberOfReviewers = 1 @@ -171,11 +157,8 @@ describe('chooseUsersFromGroups', () => { // GIVEN const owner = 'owner' const reviewers = { - groupA: [ - 'owner', - 'reviewer1' - ], - groupB: [] + groupA: ['owner', 'reviewer1'], + groupB: [], } const numberOfReviewers = 1 @@ -192,10 +175,7 @@ describe('chooseUsersFromGroups', () => { const owner = 'owner' const reviewers = { groupA: [], - groupB: [ - 'owner', - 'reviewer1' - ] + groupB: ['owner', 'reviewer1'], } const numberOfReviewers = 2 @@ -212,7 +192,7 @@ describe('chooseUsersFromGroups', () => { const owner = 'owner' const reviewers = { groupA: [], - groupB: [] + groupB: [], } const numberOfReviewers = 2 @@ -224,3 +204,86 @@ describe('chooseUsersFromGroups', () => { expect(list).toEqual([]) }) }) + +describe('chooseReviewers', () => { + test('returns separate reviewers and team_reviewers arrays', () => { + // GIVEN + const owner = 'pr-creator' + const config = { + addReviewers: true, + addAssignees: false, + reviewers: ['reviewer1', 'reviewer2', 'pr-creator'], + teamReviewers: ['team1', 'org/team2'], + assignees: [], + numberOfAssignees: 0, + numberOfReviewers: 0, + skipKeywords: [], + useReviewGroups: false, + useAssigneeGroups: false, + reviewGroups: {}, + assigneeGroups: {}, + skipUsers: [], + } + + // WHEN + const result = chooseReviewers(owner, config) + + // THEN + expect(result.reviewers).toEqual(['reviewer1', 'reviewer2']) + expect(result.team_reviewers).toEqual(['team1', 'org/team2']) + }) + + test('handles empty team_reviewers config', () => { + // GIVEN + const owner = 'pr-creator' + const config = { + addReviewers: true, + addAssignees: false, + reviewers: ['reviewer1'], + teamReviewers: [], + assignees: [], + numberOfAssignees: 0, + numberOfReviewers: 0, + skipKeywords: [], + useReviewGroups: false, + useAssigneeGroups: false, + reviewGroups: {}, + assigneeGroups: {}, + skipUsers: [], + } + + // WHEN + const result = chooseReviewers(owner, config) + + // THEN + expect(result.reviewers).toEqual(['reviewer1']) + expect(result.team_reviewers).toEqual([]) + }) + + test('respects numberOfReviewers for individual reviewers', () => { + // GIVEN + const owner = 'pr-creator' + const config = { + addReviewers: true, + addAssignees: false, + reviewers: ['reviewer1', 'reviewer2', 'reviewer3'], + teamReviewers: ['team1', 'team2'], + assignees: [], + numberOfAssignees: 0, + numberOfReviewers: 1, + skipKeywords: [], + useReviewGroups: false, + useAssigneeGroups: false, + reviewGroups: {}, + assigneeGroups: {}, + skipUsers: [], + } + + // WHEN + const result = chooseReviewers(owner, config) + + // THEN + expect(result.reviewers).toHaveLength(1) + expect(result.team_reviewers).toEqual(['team1', 'team2']) // Team reviewers are not affected by numberOfReviewers + }) +}) From 85e035824afc3399c60743b82579227543a0f02f Mon Sep 17 00:00:00 2001 From: Adrien Clerbois <50712277+AClerbois@users.noreply.github.com> Date: Wed, 13 Aug 2025 15:07:42 +0200 Subject: [PATCH 2/2] Delete CHANGEMENTS.md --- CHANGEMENTS.md | 60 -------------------------------------------------- 1 file changed, 60 deletions(-) delete mode 100644 CHANGEMENTS.md diff --git a/CHANGEMENTS.md b/CHANGEMENTS.md deleted file mode 100644 index b3df7cc..0000000 --- a/CHANGEMENTS.md +++ /dev/null @@ -1,60 +0,0 @@ -# Changements apportés pour supporter les team_reviewers séparés - -## Résumé -Ajout de la possibilité de spécifier les `team_reviewers` dans un tableau séparé des `reviewers` individuels dans la configuration. - -## Modifications apportées - -### 1. Interface Config (src/handler.ts) -- Ajout de la propriété `team_reviewers: string[]` à l'interface `Config` - -### 2. Logique de sélection des reviewers (src/util.ts) -- Modification de la fonction `chooseReviewers()` pour traiter séparément : - - `reviewers` : tableau des utilisateurs individuels (filtré pour exclure le créateur de la PR) - - `team_reviewers` : tableau des équipes (utilisé tel quel) -- Le `numberOfReviewers` n'affecte que les reviewers individuels, pas les équipes - -### 3. Documentation (README.md) -- Mise à jour de la section "Single Reviewers List" pour montrer l'utilisation de `team_reviewers` -- Suppression de l'ancienne section qui expliquait l'utilisation de la syntaxe `/team` dans `reviewers` -- Clarification que `numberOfReviewers` n'affecte que les reviewers individuels - -### 4. Tests -- Ajout de tests pour vérifier le bon fonctionnement des `team_reviewers` séparés -- Mise à jour des tests existants pour utiliser la nouvelle structure -- Tests pour les cas de configurations vides ou manquantes - -### 5. Exemple de configuration -- Création d'un fichier `auto_assign_example.yml` montrant la nouvelle syntaxe - -## Usage - -### Avant (ancienne méthode - toujours supportée pour les groupes) -```yaml -reviewers: - - reviewer1 - - reviewer2 - - /team1 - - org/team2 -``` - -### Maintenant (nouvelle méthode recommandée) -```yaml -reviewers: - - reviewer1 - - reviewer2 - -team_reviewers: - - team1 - - org/team2 -``` - -## Avantages de cette approche -1. **Séparation claire** : Les utilisateurs et équipes sont clairement séparés -2. **Contrôle granulaire** : `numberOfReviewers` n'affecte que les utilisateurs individuels -3. **Facilité de configuration** : Plus facile de configurer et comprendre -4. **Compatibilité** : Les groupes continuent de fonctionner comme avant -5. **Pas de limite** : Toutes les équipes spécifiées sont ajoutées (pas de sélection aléatoire) - -## Rétro-compatibilité -Cette modification est rétro-compatible. Si `team_reviewers` n'est pas spécifié, il sera traité comme un tableau vide et les équipes peuvent toujours être spécifiées dans `reviewers` via les groupes de review.