Flux de travail Github préféré pour la mise à jour d'une demande d'extraction après examen du code

341

J'ai soumis une modification à un projet Open Source sur Github et reçu des commentaires de révision de code d'un des membres de l'équipe principale.

Je voudrais mettre à jour le code en tenant compte des commentaires de révision et le soumettre à nouveau. Quel est le meilleur flux de travail pour ce faire? De ma connaissance limitée de git / github, je pourrais faire l'une des choses suivantes:

  1. Mettez à jour le code en tant que nouveau commit et ajoutez à la fois le commit initial et le commit mis à jour à ma demande d'extraction.

  2. D'une manière ou d'une autre (??) restaurer l'ancien commit de mon référentiel, et créer un seul nouveau commit contenant tout, puis lancer une requête pull pour cela?

  3. git commita une fonctionnalité de modification, mais j'ai entendu dire que vous ne devriez pas l'utiliser après avoir poussé la validation en dehors de votre référentiel local? Dans ce cas, j'ai effectué le changement sur mon PC local et poussé vers ma branche github du projet. Serait-ce correct d'utiliser «modifier»?

  4. Autre chose?

Il semble que l'option 2/3 serait bien, car le projet open source n'aurait qu'un seul commit dans son histoire qui implémenterait tout, mais je ne sais pas comment faire.

Remarque: je ne sais pas si cela affecte la réponse ou non, mais je n'ai pas apporté les modifications dans une branche distincte, je viens de faire un commit sur le maître

Orion Edwards
la source

Réponses:

219

Ajoutez simplement un nouveau commit à la branche utilisée dans la demande d'extraction et poussez la branche vers GitHub. La demande d'extraction sera automatiquement mise à jour avec la validation supplémentaire.

Les numéros 2 et 3 ne sont pas nécessaires. Si les gens veulent voir uniquement où votre branche a été fusionnée (et non les git log --first-parentvalidations supplémentaires), ils peuvent utiliser pour afficher uniquement la validation de fusion dans le journal.

ambre
la source
7
masterest aussi une branche, donc techniquement, cela n'a pas d'importance :)
poke
10
@OrionEdwards - comme mentionné, le maître est une branche, donc, la mettre à jour entraînera également la mise à jour de toutes les demandes d'extraction basées sur elle. (C'est une bonne raison d'utiliser une branche distincte pour tout ce que vous prévoyez de soumettre une demande de pull.)
Amber
18
Étant donné que le code est toujours en cours de révision, il est généralement préférable de corriger les validations plutôt que d'introduire des validations de correction supplémentaires qui encombrent simplement l'historique ...
mgalgs
4
@mgalgs C'est une question de préférence.
Amber
4
Je n'aime pas cette réponse pour des raisons expliquées dans le blog que je viens d'écrire ; Je pense que l'autre réponse est bien meilleure.
Adam Spires
225

Pour mettre à jour une demande d'extraction

Pour mettre à jour une demande d'extraction (point # 1), la seule chose que vous devez faire est d'extraire la même branche d'où provient la demande d'extraction et de la pousser à nouveau:

cd /my/fork
git checkout master
...
git commit -va -m "Correcting for PR comments"
git push

Facultatif - Nettoyage de l'historique des validations

Vous pouvez être invité à écraser vos validations ensemble afin que l'historique du référentiel soit propre, ou vous-même souhaitez supprimer les validations intermédiaires qui distraient du "message" dans votre demande de tirage (point n ° 2). Par exemple, si votre historique de commit ressemble à ceci:

$ git remote add parent [email protected]:other-user/project.git
$ git fetch parent
$ git log --oneline parent/master..master
e4e32b8 add test case as per PR comments
eccaa56 code standard fixes as per PR comments
fb30112 correct typos and fatal error
58ae094 fixing problem

C'est une bonne idée d'écraser les choses ensemble pour qu'elles apparaissent comme un seul commit:

$ git rebase -i parent/master 

Cela vous invitera à choisir comment réécrire l'historique de votre demande de pull, les éléments suivants seront dans votre éditeur:

pick 58ae094 fixing actual problem
pick fb30112 correct typos
pick eccaa56 code standard fixes
pick e4e32b8 add test case as per PR comments

Pour tout commit que vous souhaitez faire partie du commit précédent - changez le choix en squash:

pick 58ae094 fixing actual problem
squash fb30112 correct typos
squash eccaa56 code standard fixes
squash e4e32b8 add test case as per PR comments

Et fermez votre éditeur. Git réécrira alors l'historique et vous invitera à fournir un message de validation pour la validation combinée. Modifiez en conséquence et votre historique de validation sera désormais concis:

$ git log --oneline parent/master..master
9de3202 fixing actual problem

Poussez ça sur votre fourche:

$ git push -f
Counting objects: 19, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (5/5), done.
Writing objects: 100% (11/11), 978 bytes, done.
Total 11 (delta 9), reused 7 (delta 6)
To [email protected]:me/my-fork.git
   f1238d0..9de3202  HEAD -> master

et votre demande d'extraction contiendra un seul commit, incorporant toutes les modifications précédemment divisées en plusieurs commits.

Changer l'historique des dépôts publics est une mauvaise chose

Réécrire l'historique et l'utiliser git push -fsur une branche que quelqu'un d'autre a peut-être déjà clonée est une mauvaise chose - cela fait différer l'historique du référentiel et celui de la caisse.

Cependant, la modification de l'histoire de la fourche pour corriger le changement que vous proposez d'être intégré dans un dépôt - est une bonne chose. En tant que tel, vous n'avez aucune réserve à éliminer le "bruit" de vos demandes de tirage.

Une note sur les branches

Dans ce qui précède, je montre que la demande de tirage provient de la masterbranche de votre fork, il n'y a rien de mal à cela nécessairement, mais cela crée certaines limitations telles que, si c'est votre technique standard, ne pouvant avoir qu'un seul PR ouvert par référentiel . Il est préférable de créer une branche pour chaque changement individuel que vous souhaitez proposer:

$ git branch feature/new-widgets
$ git checkout feature/new-widgets
...
Hack hack hack
...
$ git push
# Now create PR from feature/new-widgets
AD7six
la source
28
+1 pour avoir mentionné comment nettoyer les validations plutôt que de pousser des validations de correction supplémentaires.
mgalgs
3
J'ai rencontré des problèmes de cueillette / écrasement et cette réponse m'a aidé. A également remarqué que Github a supprimé la conversation précédente après moi git push -f. Il n'y a pas eu beaucoup de commentaires, mais c'est quelque chose à quoi je ne m'attendais pas.
Hitesh
5
Juste pour être clair, lorsque vous rebasez pour avoir une histoire propre, vous changez en effet vos engagements publics, vous supposez simplement que personne ne s'en soucie parce que c'est une fourchette.
brita_
2
Suivi: meilleures pratiques lorsque le maître change pendant votre RP?
Kevin Suttle
1
Considérez que la réécriture de l'historique des demandes d'extraction qui ont été examinées (ou qui ont en général des commentaires sur / faisant référence au code) peut prêter à confusion, car l'historique ne correspondra plus aux commentaires auxquels il faisait référence. Il n'y a pas de solution facile: quelqu'un fermera le PR et le référera dans un nouveau (pour ne pas réécrire l'historique); mon idée serait de simplement sauvegarder le dernier SHA de validation qui est en cours de réinitialisation / réécriture et de le référer dans un commentaire sur le PR, après que le push forcé ait été effectué. SI prune ne supprime pas cette validation détachée, son historique correspondra toujours aux commentaires de PR.
Kamafeather