6 ms·

It’s actually very convenient now to stack GitHub PRs; the target branch detector makes it easy to arrange a cascade, and the target branches update automatical

It’s actually very convenient now to stack GitHub PRs; the target branch detector makes it easy to arrange a cascade, and the target branches update automatically as the earlier ones merge. Can you explain what a patch based workflow means? I’ve seen patches in email. But to be honest I really only know Github. What I would say in my apparent ignorance is that a patch doesn’t exist in isolation; it has, or should have, a parent — a certain code base state explicitly declared as its context. Otherwise the entire activity of review is not well-defined, since no-one has specified what the state of the code base is, and therefore the behavior of the software is unknowable.

bonzini 6y ago
Patches are sent as emails with a format like Commit message -- Diffstat Diff And reviews happen simply by replying to the email. If the maintainer is satisfied the email can be piped in a program that applies the patch. Most programs and people that use email for patches have pretty sophisticated tooling built around it. See for example a series of four patches (i.e. a pull requests consisting of four commits) in Patchew: https://patchew.org/QEMU/20210415130305.28640-1-iii@linux.ibm.com/ https://patchew.org/QEMU/20210415130305.28640-1-iii@linux.ib... Notably, Patchew can provide a diff-of-diffs view that isn't there in either GitHub or Gitlab: https://patchew.org/QEMU/20210415130305.28640-1-iii@linux.ibm.com/diff/20210416154939.32404-1-iii@linux.ibm.com/ https://patchew.org/QEMU/20210415130305.28640-1-iii@linux.ib... (Patchew was started by a colleague of mine at Red Hat).
- da39a3ee 6y ago
  Using email to review patches is completely absurd. The following is a contentious statement but there's no avoiding it: it's done nowadays by people who started doing it in the 80s and 90s and are being stubborn. Here are some reasons email should not be used for review, if any are really needed: - A "patch" must have a defined parent. Otherwise it is not defined what state of the overall codebase is being reviewed. - Proposed changes evolve through complex lifecycles of comments/discussions and updates. Some comments are global and some should be attached to specific lines of code as it exists in a specific revision. This complexity is (obviously!) better modeled by a dedicated UI than by shoe-horning it into plain text email. - It makes sense to attach the dynamic status of test runs to these updates. This can be done in a web UI but cannot be done in an email. - Commenting on individual lines of code is desirable and (obviously!) better done by attaching a discussion thread to a defined (revision + code coordinates) location in a diff view than by replying inline in an email. The reasons (if they really need stating) include (1) that breaking a diff manually in an email and replying inline makes it hard to assemble the diff in your head, (2) it is not possible to have clearly readable discussion threads involving multiple people with each thread attached to a certain line in the diff, (3) it's certainly not possible to keep that discussion coherent when the underlying patch is evolving. - Not everyone uses, or wishes to use, plain text emails. - Not everyone uses or wishes to use a text-based email client that runs on their local machine. Nor do they have, or wish to have, their emails present on their local machine (they want their emails stored online and accessed via a web interface). (And yes, I am someone who has used mutt, and gnus, and dovecot, and offlineimap.) - The history/evolution of a codebase can be represented by a graph structure. It makes sense for review discussion to be associated with this graph structure. This is what Bitbucket/Github/Gitlab provide. Email does not provide this. There's a risk with email that it won't even be publicly searchable, but supposing it exists, a web-hosted archive of emails is not an effective way to comprehend the history of a codebase. As a cultural observation, the people who are still insisting on email are often the sorts of people who insist that email should be plain text and actually look down upon technical people who like web-hosted email clients and HTML emails, and they are sometimes the sorts of people who deride Github as being nonserious -- full of "kids" who carelessly propose ill-thought out change sets and expect them to be incorporated into public projects. Neither of those positions are reasonable or speak highly of the holder.
  - lima 6y ago
    Note that those points are particular disadvantages of the email workflow - all of these things are solved in modern tools like Gerrit.
    - da39a3ee 6y ago
      If you don't know what the parent is in Gerrit then how can you run CI on the patch? And if you do know what the parent is then how is it different from a "PR"?
      - lima 6y ago
        A PR is roughly the same as a stack of Gerrit CLs. The difference is that Gerrit tracks the evolution of, and reviews, each individual commit. With a PR, you lose history when rebasing and all commits have to be approved as a whole.
        da39a3ee 6y ago
        > The difference is that Gerrit tracks the evolution of, and reviews, each individual commit. What does it mean for an individual commit to evolve? I think of commits as being the atoms of the evolutionary process; i.e. evolution is a sequence of commits. > With a PR, you lose history when rebasing I don't quite understand that point. (I know what rebasing is.) > and all commits have to be approved as a whole. Hm, so why would one want to approve an individual "commit"? In my world a commit should be a meaningful change but isn't necessarily self-contained and hence not approvable. This sounds like what you're calling commits are in fact independent / self-contained and deployable units of change. So then > A PR is roughly the same as a stack of Gerrit CLs. Does a "CL" contain one or more than one of the entities you refer to as "commits"above? Isn't it the other way around? A Gerrit thing is a stack of mini "PR"s, where each "PR" is a small, but independent / self-contained and deployable, unit of work. In any case, a PR contains many non-self-contained units (commits) so I'm having a hard time equating a PR and a stack of CLs.
lima 6y ago
> It’s actually very convenient now to stack GitHub PRs More convenient than before, but nowhere near as convenient as keeping everything on a single branch so that it can be rebased in one go.

bonzini 6y ago

Patches are sent as emails with a format like Commit message -- Diffstat Diff And reviews happen simply by replying to the email. If the maintainer is satisfied the email can be piped in a program that applies the patch. Most programs and people that use email for patches have pretty sophisticated tooling built around it. See for example a series of four patches (i.e. a pull requests consisting of four commits) in Patchew: https://patchew.org/QEMU/20210415130305.28640-1-iii@linux.ibm.com/ https://patchew.org/QEMU/20210415130305.28640-1-iii@linux.ib... Notably, Patchew can provide a diff-of-diffs view that isn't there in either GitHub or Gitlab: https://patchew.org/QEMU/20210415130305.28640-1-iii@linux.ibm.com/diff/20210416154939.32404-1-iii@linux.ibm.com/ https://patchew.org/QEMU/20210415130305.28640-1-iii@linux.ib... (Patchew was started by a colleague of mine at Red Hat).

da39a3ee 6y ago
Using email to review patches is completely absurd. The following is a contentious statement but there's no avoiding it: it's done nowadays by people who started doing it in the 80s and 90s and are being stubborn. Here are some reasons email should not be used for review, if any are really needed: - A "patch" must have a defined parent. Otherwise it is not defined what state of the overall codebase is being reviewed. - Proposed changes evolve through complex lifecycles of comments/discussions and updates. Some comments are global and some should be attached to specific lines of code as it exists in a specific revision. This complexity is (obviously!) better modeled by a dedicated UI than by shoe-horning it into plain text email. - It makes sense to attach the dynamic status of test runs to these updates. This can be done in a web UI but cannot be done in an email. - Commenting on individual lines of code is desirable and (obviously!) better done by attaching a discussion thread to a defined (revision + code coordinates) location in a diff view than by replying inline in an email. The reasons (if they really need stating) include (1) that breaking a diff manually in an email and replying inline makes it hard to assemble the diff in your head, (2) it is not possible to have clearly readable discussion threads involving multiple people with each thread attached to a certain line in the diff, (3) it's certainly not possible to keep that discussion coherent when the underlying patch is evolving. - Not everyone uses, or wishes to use, plain text emails. - Not everyone uses or wishes to use a text-based email client that runs on their local machine. Nor do they have, or wish to have, their emails present on their local machine (they want their emails stored online and accessed via a web interface). (And yes, I am someone who has used mutt, and gnus, and dovecot, and offlineimap.) - The history/evolution of a codebase can be represented by a graph structure. It makes sense for review discussion to be associated with this graph structure. This is what Bitbucket/Github/Gitlab provide. Email does not provide this. There's a risk with email that it won't even be publicly searchable, but supposing it exists, a web-hosted archive of emails is not an effective way to comprehend the history of a codebase. As a cultural observation, the people who are still insisting on email are often the sorts of people who insist that email should be plain text and actually look down upon technical people who like web-hosted email clients and HTML emails, and they are sometimes the sorts of people who deride Github as being nonserious -- full of "kids" who carelessly propose ill-thought out change sets and expect them to be incorporated into public projects. Neither of those positions are reasonable or speak highly of the holder.
- lima 6y ago
  Note that those points are particular disadvantages of the email workflow - all of these things are solved in modern tools like Gerrit.
  - da39a3ee 6y ago
    If you don't know what the parent is in Gerrit then how can you run CI on the patch? And if you do know what the parent is then how is it different from a "PR"?
    - lima 6y ago
      A PR is roughly the same as a stack of Gerrit CLs. The difference is that Gerrit tracks the evolution of, and reviews, each individual commit. With a PR, you lose history when rebasing and all commits have to be approved as a whole.
      - da39a3ee 6y ago
        > The difference is that Gerrit tracks the evolution of, and reviews, each individual commit. What does it mean for an individual commit to evolve? I think of commits as being the atoms of the evolutionary process; i.e. evolution is a sequence of commits. > With a PR, you lose history when rebasing I don't quite understand that point. (I know what rebasing is.) > and all commits have to be approved as a whole. Hm, so why would one want to approve an individual "commit"? In my world a commit should be a meaningful change but isn't necessarily self-contained and hence not approvable. This sounds like what you're calling commits are in fact independent / self-contained and deployable units of change. So then > A PR is roughly the same as a stack of Gerrit CLs. Does a "CL" contain one or more than one of the entities you refer to as "commits"above? Isn't it the other way around? A Gerrit thing is a stack of mini "PR"s, where each "PR" is a small, but independent / self-contained and deployable, unit of work. In any case, a PR contains many non-self-contained units (commits) so I'm having a hard time equating a PR and a stack of CLs.

lima 6y ago

> It’s actually very convenient now to stack GitHub PRs More convenient than before, but nowhere near as convenient as keeping everything on a single branch so that it can be rebased in one go.