Ajout d'un bot d'automerge #4507

jdauphant · 2020-11-03T22:18:32Z

🙂

Pour la review :

Les tests passent (avec un check vert)

Mettre le tag "automerge" pour merger cette PR

Morendil

Je ne suis pas à l'aise pour plusieurs raisons:

on merge déjà des PR qui contiennent des erreurs flagrantes, c'est donc le processus de relecture par des humains qui me semble devoir être amélioré; l'automatisation va amplifier ce qu'on fait mal, et risque d'apporter des risques spécifiques supplémentaires (notamment quels sont les impacts sur la sécurité ?)
notre stratégie a jusqu'ici consisté à responsabiliser les personnes de la communauté pour qu'elles deviennent autonomes sur Github; l'automatisation revient à abandonner cette stratégie, et il me semble que ça doit faire l'objet d'une discussion préalable

jdauphant · 2020-11-04T10:26:34Z

@Morendil Tu as vu comment le bot fonctionne ?
C'est pour les "up to date" à faire, ça ne remplace pas la review.
Au lieu de faire "merge", tu mets "automerge" et le bot s'occupe d'update et de merge (ça me ferait gagner un temps fou).
Il n'aurait qu'une seule différence avec la situation actuelle : on n'aura pas besoins de se taper de cliquer 2 ou 3 fois sur "Update" quand on sait qu'on veut merger.

jdauphant · 2020-11-09T12:28:27Z

Bon ça ne déclenche pas les tests, c'est une des limitations des githubs actions :(

jdauphant · 2020-11-20T17:52:21Z

Pix m'a donné la solution, il faut un compte avec un bot

LucasCharrier · 2021-01-20T16:51:40Z

@jdauphant,
Je ne maitrise pas bien les github actions et je ne suis pas sûr de ce que tu veux dire par "ne déclenche pas les tests". Ça ne les déclenche pas ou ça n'attend pas qu'ils tournent pour merge ?
Ils ont updaté le README du github action suite a cette issue qui semble similaire : pascalgn/automerge-action#137
Aussi j'ai regardé les events qui trigger l'action d'automerge, et je me demande s'il n'y a pas moyen d'utilisé l'event 'workflow_runs' :
https://docs.github.com/en/actions/reference/events-that-trigger-workflows#workflow_run

jdauphant · 2021-01-20T19:25:30Z

L'automerge est une github action, ça créé un commit.
Les tests sont un autre github action.
Un commit d'une github action ne peut pas déclencher automatiquement une autre action apparemment.

Dans le readme de automerge-action, il dise "When a pull request is merged by this action, the merge will not trigger other GitHub workflows. Similarly, when another GitHub workflow creates a pull request, this action will not be triggered. This is because an action in a workflow run can't trigger a new workflow run. However, the workflow_run event is triggered as expected."
Je ne connais pas workflow_run, peut-être que ça résou le problème

LucasCharrier · 2021-01-21T07:45:12Z

Ok merci c'est plus clair. Du coup peut être en rajoutant une condition worflow_run dans le workflow Tests :

on:
  push:
    branches: [ master ]
  pull_request:
    branches: [ master ]
 workflow_run:
    workflows: ["automerge"]
    branches: [master]
    types: 
      - completed

ça voudrait dire que ça déclenche le tests soit sur push, soit sur pull request soit quand le workflows automerge est completed

jdauphant · 2021-01-21T13:17:54Z

N'hésites pas à tester sur cette PR ou une autre. Si tu arrives à automerge une la banche qui fait ça, c'est un bon départ ^^

LucasCharrier · 2021-01-21T14:02:40Z

Alright je vais test avec ta branche

This reverts commit 70d2567.

LucasCharrier · 2021-01-21T14:48:41Z

@jdauphant Donc ça ne fait pas l'update branche tout seul, par contre ça merge bien tout seul la pull request si la branche est up to date et si on attend bien que les checks soient fait avec un MERGE_RETRY_SLEEP adhoc.
J'ai fait un revert en attendant.
Ce que je ne comprends pas c'est qu'il précise dans la doc :
Changes from the base branch will automatically be merged into the pull request (only when "Require branches to be up to date before merging" is enabled in the branch protection rules), donc l'up to date devrait être fait automatiquement.

This reverts commit 70d2567.

LucasCharrier · 2021-01-21T14:58:44Z

Ok j'ai compris.
Quand tu as testé, il faisait bien le up to date, mais ne lançait pas les tests derriéres et donc ne faisait jamais l'automerge.

LucasCharrier · 2021-01-21T15:16:31Z

Alors pour résumé :

Le label ayant été changé sur notre github par "merge it !" et étant par défaut "automerge" c'est pour ça que l'update ne se faisait pas. J'avais par contre bien mis "merge it !" pour le merge, ce qui explique qu'il fonctionnait.
A part si quelque chose m'échappe, l'automerge marche avec le fichier automerge de cette branche : https://github.com/betagouv/beta.gouv.fr/tree/task/add-automerge
Il suffit de faire la PR et de mettre le label 'merge it !'

jdauphant · 2021-01-21T22:22:02Z

Ok on peut merge ça sur master et voir ce que ça donne ? (j'ai taggé des PRs)

LucasCharrier · 2021-01-22T08:56:22Z

@jdauphant, j'avais du faire dans mes tests une action qui redéclenchait les tests. Tant pis j'abandonne pour l'instant cette github action et je vais revert le commit en question. Je jetterai un coup d'oeil à la solution de pix, maintenant que j'ai bien cerné les mécanismes.

LucasCharrier · 2021-01-22T12:51:13Z

@jdauphant j'ai jeté un oeil a la solution de pix.

Il s'agirait de créer un utilisateur bot type secretaria-bot, de générer un token github, et d'utliser ce token à la place du secret.GITHUB_ACTION ce qui aurait pour effet de relancer les tests puisque l'automerge sera considérée comme une action utilisateur.
https://github.com/1024pix/pix/pull/1623/files
Je ne peux pas le faire de mon coté car il faut l'accès aux settings du repo pour mettre la valeur du token.

Je ne sais pas si c'est la meilleurs solution,
autrement le workflow_run devrait pouvoir marcher, mais à retester. Mes tests ont été pour l'instant infructueux. J'ai référencé les différents tests dans l'issue : #5085

jdauphant · 2021-01-22T12:53:16Z

Je viens de mettre admin, il doit falloir créer un compte pour le bot

LucasCharrier · 2021-01-22T12:59:07Z

Ok 👍 , oui en effet il faut créer un compte, a moins d'utiliser celui de secrétariat ?

LucasCharrier · 2021-01-22T13:30:03Z

@jdauphant, en effet ça marche. J'ai testé avec un token lié à mon compte. A voir du coup si on crée un compte bot spécial, si on utilise celui de secrétariat par exemple. Ça me semble pas idéal de multiplier les comptes.

jdauphant · 2021-01-22T13:32:41Z

Je ne serais pas à la réutilisation, on n'a fait exprès de ne pas donner les droits d'écrire sur le repo au bot secretariat. C'est mieux en terme de gestion de sécurité de ne pas réutiliser.
Par contre, on n'a pas d'espace pour partager les crédentials.

LucasCharrier · 2021-01-22T13:50:53Z

Ok, oui c'était pour éviter d'avoir plusieurs compte "bot", avec des crédentials par ci, par là.
On peut utiliser un token "perso" aussi.

Hum, du coup le plus propre si on crée un bot à part, c'est un compte avec une adresse beta.gouv et pour les crédentials, des services comme dashlane ou équivalent permettent de faire ce genre de partage.

jdauphant · 2021-01-22T13:57:11Z

Pas de tokens perso aussi, ça donne beaucoup de droit.

Morendil · 2021-01-22T15:04:11Z

Ca me semblerait plus logique que cette discussion ait lieu dans #5085 plutôt que ici dans une PR qui est déjà mergée…

Ajout d'un bot d'automerge

6ef203d

Morendil suggested changes Nov 4, 2020

View reviewed changes

Flightan approved these changes Nov 9, 2020

View reviewed changes

jdauphant added the automerge label Nov 9, 2020

github-actions bot and others added 3 commits November 9, 2020 09:17

Merge master into task/add-automerge

6a22e7c

Update automerge.yml

9dba215

Merge master into task/add-automerge

bad0ccf

jdauphant closed this Nov 20, 2020

Change automerge label

3e79b7e

LucasCharrier reopened this Jan 21, 2021

LucasCharrier and others added 5 commits January 21, 2021 15:20

Added only check suite

f9d35c1

Added automerge after test

ed40193

Fixed syntaxe

1f8d0a9

Clean branch after identifying problem

f17cc25

Merge branch 'master' into task/add-automerge

5775c29

github-actions bot merged commit 70d2567 into master Jan 21, 2021

github-actions bot deleted the task/add-automerge branch January 21, 2021 14:44

LucasCharrier added a commit that referenced this pull request Jan 21, 2021

Revert "Ajout d'un bot d'automerge (#4507)"

8c6bf2d

This reverts commit 70d2567.

LucasCharrier mentioned this pull request Jan 21, 2021

Revert "Ajout d'un bot d'automerge" #5052

Merged

LucasCharrier restored the task/add-automerge branch January 21, 2021 14:50

LucasCharrier added a commit that referenced this pull request Jan 21, 2021

Revert "Ajout d'un bot d'automerge (#4507)" (#5052)

e3017ab

This reverts commit 70d2567.

LucasCharrier mentioned this pull request Jan 22, 2021

Ajout d'un bot d'automerge #5085

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ajout d'un bot d'automerge #4507

Ajout d'un bot d'automerge #4507

jdauphant commented Nov 3, 2020 •

edited

Loading

Morendil left a comment

jdauphant commented Nov 4, 2020 •

edited

Loading

jdauphant commented Nov 9, 2020 •

edited

Loading

jdauphant commented Nov 20, 2020

LucasCharrier commented Jan 20, 2021 •

edited

Loading

jdauphant commented Jan 20, 2021

LucasCharrier commented Jan 21, 2021 •

edited

Loading

jdauphant commented Jan 21, 2021

LucasCharrier commented Jan 21, 2021

LucasCharrier commented Jan 21, 2021 •

edited

Loading

LucasCharrier commented Jan 21, 2021

LucasCharrier commented Jan 21, 2021

jdauphant commented Jan 21, 2021

LucasCharrier commented Jan 22, 2021 •

edited

Loading

LucasCharrier commented Jan 22, 2021 •

edited

Loading

jdauphant commented Jan 22, 2021

LucasCharrier commented Jan 22, 2021

LucasCharrier commented Jan 22, 2021 •

edited

Loading

jdauphant commented Jan 22, 2021 •

edited

Loading

LucasCharrier commented Jan 22, 2021

jdauphant commented Jan 22, 2021

Morendil commented Jan 22, 2021

Ajout d'un bot d'automerge #4507

Ajout d'un bot d'automerge #4507

Conversation

jdauphant commented Nov 3, 2020 • edited Loading

Morendil left a comment

Choose a reason for hiding this comment

jdauphant commented Nov 4, 2020 • edited Loading

jdauphant commented Nov 9, 2020 • edited Loading

jdauphant commented Nov 20, 2020

LucasCharrier commented Jan 20, 2021 • edited Loading

jdauphant commented Jan 20, 2021

LucasCharrier commented Jan 21, 2021 • edited Loading

jdauphant commented Jan 21, 2021

LucasCharrier commented Jan 21, 2021

LucasCharrier commented Jan 21, 2021 • edited Loading

LucasCharrier commented Jan 21, 2021

LucasCharrier commented Jan 21, 2021

jdauphant commented Jan 21, 2021

LucasCharrier commented Jan 22, 2021 • edited Loading

LucasCharrier commented Jan 22, 2021 • edited Loading

jdauphant commented Jan 22, 2021

LucasCharrier commented Jan 22, 2021

LucasCharrier commented Jan 22, 2021 • edited Loading

jdauphant commented Jan 22, 2021 • edited Loading

LucasCharrier commented Jan 22, 2021

jdauphant commented Jan 22, 2021

Morendil commented Jan 22, 2021

jdauphant commented Nov 3, 2020 •

edited

Loading

jdauphant commented Nov 4, 2020 •

edited

Loading

jdauphant commented Nov 9, 2020 •

edited

Loading

LucasCharrier commented Jan 20, 2021 •

edited

Loading

LucasCharrier commented Jan 21, 2021 •

edited

Loading

LucasCharrier commented Jan 21, 2021 •

edited

Loading

LucasCharrier commented Jan 22, 2021 •

edited

Loading

LucasCharrier commented Jan 22, 2021 •

edited

Loading

LucasCharrier commented Jan 22, 2021 •

edited

Loading

jdauphant commented Jan 22, 2021 •

edited

Loading