How to write the perfect pull request (2015)

Re:How to write the perfect pull request (2015) by matheusmoreira on Tuesday February 18, 2020 @09:44PM (#22362647)

Better support for pull request revision would be nice. In mailing lists, the entire patch set is resent when revisions are made. When I force push revised commits to my branch, the old commits are eventually garbage collected despite the existence of references to those commits in the pull request.

Also, it is important to discuss pull request etiquette. When is it appropriate to mention the maintainers? How to draw attention to a pull request?

[ Parent | on HN ]

Re:How to write the perfect pull request (2015) by u801e on Wednesday February 19, 2020 @08:45AM (#22365332)

> Better support for pull request revision would be nice

Github added a force push diff link feature not too long ago which shows the diff after doing a force push. Unfortunately, they don't show a per commit diff or a diff of the commit messages.

[ Parent | on HN ]

Re:How to write the perfect pull request (2015) by chenhan on Tuesday February 18, 2020 @09:48PM (#22362664)

It is a problem in mailing lists but GitHub has good tooling around it. Every time you force-push, GitHub keeps and provides direct links to the old commit, the new commit and the diff between the two commits. Every force push appears as its own diff on the GitHub PR page.

How is GitLab doing in this area? Does GitLab show diffs for force-pushes?

[ Parent | on HN ]

Re:How to write the perfect pull request (2015) by matheusmoreira on Tuesday February 18, 2020 @09:59PM (#22362706)

> Every time you force-push, GitHub keeps and provides direct links to the old commit, the new commit and the diff between the two commits.

Not in my experience. Here's one of my pull requests:

https://github.com/mchehab/zbar/pull/64

The maintainer reviewed some of the changes and I revised my commits as a result. The review is correctly marked as outdated. Clicking on the file name tells me the commit cannot be found.

> We went looking everywhere, but couldn’t find those commits.

> Sometimes commits can disappear after a force-push. Head back to the latest changes here.

[ Parent | on HN ]

Re:How to write the perfect pull request (2015) by chenhan on Tuesday February 18, 2020 @10:01PM (#22362713)

Your pull request shows exactly what I am talking about. For every force-push it has direct links to the old commit, the new commit and the diff.

Pick the first force push in your PR. It says

> matheusmoreira force-pushed the matheusmoreira:binary-decoding branch from aec04b3 to 87a0b3c on 5 Nov 2019

Click on 'force-pushed'. That's the diff of force-push.

Click on 'aec04b3'. That's the commit before force-push.

Click on '87a0b3c'. That's the new commit in the force-push.

[ Parent | on HN ]

Re:How to write the perfect pull request (2015) by matheusmoreira on Tuesday February 18, 2020 @10:20PM (#22362789)

You're right. I never realized those messages contained links. They do lead me to the old commits.

I don't understand why the code review can't find the commit though.

[ Parent | on HN ]

Re:How to write the perfect pull request (2015) by u801e on Wednesday February 19, 2020 @08:50AM (#22365375)

> For every force-push it has direct links to the old commit, the new commit and the diff.

It's there a way to see the diff between commits that are/were not at the HEAD of the branch? That is, doing something like:

  git diff aec04b3^.. 87a0b3c^

[ Parent | on HN ]