6 ms·

My simple suggestion to my teams: PRs are emails to your team and to your future self. Framed in that context it's easier to carry the correct tone and think

My simple suggestion to my teams: PRs are emails to your team and to your future self. Framed in that context it's easier to carry the correct tone and think about scoping / what's important. --- > pedantically apply DRY to every situation I swear DRY has done more damage to the software industry from the developer side than it has done good because it has manifested into this big stick with which to bludgeon people without taking context into account.

BobbyJo 1y ago
A great way to frame DRY that I heard from hackernews: "DRY things that are supposed to have the same behavior, not things that happen to have the same behavior"
- collingreen 1y ago
  This is a really good way to put this. The "just because the do they same thing right now doesn't mean they _do the same thing_" concept is hard to convey!
- tetha 1y ago
  I enjoy Sandi Metz' point there as well: Code just looking the same is not enough to call it duplication. Once you have to change two places looking the same to add a new feature or to fix a bug, then you have duplication and should centralize it.
  - michaelcampbell 1y ago
    I usually wait till 3; 2 is about the _very general_ point at which changing multiple places is about the same work as changing it to be centralized. 3 is almost always a better place to make that leap.
m000 1y ago
> PRs are emails to your team and to your future self. This should be commits though. Typically, developers would look for clues in this order: code -> code comment -> commit message -> PR text -> external document So commit messages puts the information closer to the user. One hop doesn't seem much, but the time saved adds up as you go. Also, as some other reader mentioned anecdotally, PRs may not be there forever. E.g. your team may migrate to a new platform PR text and reviews were left behind.
- herval 1y ago
  In most sane development cycles I've seen (from 2-people teams to 100k people teams), intermediate commits disappear as soon as you merge your branch (in other words, you do short lived branches + squash merge). If you decide to do merges without squashing, then yes, you gotta have to have more hygiene on each individual commit. It creates a lot of unnecessary friction and it's guaranteed to be slower (devs can't use commits as checkpoints/savepoints on their work, but rather each commit becomes a fully fleshed out "intermediate final state"). The only situation where I see this making sense is if you share work on a branch with other engineers (which is also a bad idea).
  - m000 1y ago
    > devs can't use commits as checkpoints/savepoints on their work But they can! In git you can do whatever you want with your local/remote working branch. And after you're done it's pretty straightforward to massage it into a coherent series of commits (especially if you had been working with that in mind). > each commit becomes a fully fleshed out "intermediate final state" This is really a team decision. You can allow intermediate commits to e.g. fail the tests, and add a tag to your main/master after each merge. Then you know that only the tagged commits are guaranteed to be fully functional.
    - herval 1y ago
      > And after you're done it's pretty straightforward to massage it into a coherent series of commits Why waste time? Just squash and merge, you have a single commit and it WORKS. Intermediate messages disappear and you have a single, atomic rollback point on your main branch > You can allow intermediate commits to e.g. fail the tests, and add a tag to your main/master after each merge. Then you know that only the tagged commits are guaranteed to be fully functional. OR… squash and merge. Block merging with tests and compilation passing For anything in tech, there’s the frictionless way and the busywork way. Both of your examples are busywork that’s completely unnecessary if you just… squash and merge The best process is the process nobody needs to remember to do shit for it to work
bear8642 1y ago
> PRs are emails to your team and to your future self. Indeed! I've found many point on this discussion answered by the linux kernel idea of mailing lists where a change is discussed then approved, often with feedback acknowledged

BobbyJo 1y ago

A great way to frame DRY that I heard from hackernews: "DRY things that are supposed to have the same behavior, not things that happen to have the same behavior"

collingreen 1y ago
This is a really good way to put this. The "just because the do they same thing right now doesn't mean they _do the same thing_" concept is hard to convey!
tetha 1y ago
I enjoy Sandi Metz' point there as well: Code just looking the same is not enough to call it duplication. Once you have to change two places looking the same to add a new feature or to fix a bug, then you have duplication and should centralize it.
- michaelcampbell 1y ago
  I usually wait till 3; 2 is about the _very general_ point at which changing multiple places is about the same work as changing it to be centralized. 3 is almost always a better place to make that leap.

m000 1y ago

> PRs are emails to your team and to your future self. This should be commits though. Typically, developers would look for clues in this order: code -> code comment -> commit message -> PR text -> external document So commit messages puts the information closer to the user. One hop doesn't seem much, but the time saved adds up as you go. Also, as some other reader mentioned anecdotally, PRs may not be there forever. E.g. your team may migrate to a new platform PR text and reviews were left behind.

herval 1y ago
In most sane development cycles I've seen (from 2-people teams to 100k people teams), intermediate commits disappear as soon as you merge your branch (in other words, you do short lived branches + squash merge). If you decide to do merges without squashing, then yes, you gotta have to have more hygiene on each individual commit. It creates a lot of unnecessary friction and it's guaranteed to be slower (devs can't use commits as checkpoints/savepoints on their work, but rather each commit becomes a fully fleshed out "intermediate final state"). The only situation where I see this making sense is if you share work on a branch with other engineers (which is also a bad idea).
- m000 1y ago
  > devs can't use commits as checkpoints/savepoints on their work But they can! In git you can do whatever you want with your local/remote working branch. And after you're done it's pretty straightforward to massage it into a coherent series of commits (especially if you had been working with that in mind). > each commit becomes a fully fleshed out "intermediate final state" This is really a team decision. You can allow intermediate commits to e.g. fail the tests, and add a tag to your main/master after each merge. Then you know that only the tagged commits are guaranteed to be fully functional.
  - herval 1y ago
    > And after you're done it's pretty straightforward to massage it into a coherent series of commits Why waste time? Just squash and merge, you have a single commit and it WORKS. Intermediate messages disappear and you have a single, atomic rollback point on your main branch > You can allow intermediate commits to e.g. fail the tests, and add a tag to your main/master after each merge. Then you know that only the tagged commits are guaranteed to be fully functional. OR… squash and merge. Block merging with tests and compilation passing For anything in tech, there’s the frictionless way and the busywork way. Both of your examples are busywork that’s completely unnecessary if you just… squash and merge The best process is the process nobody needs to remember to do shit for it to work

bear8642 1y ago

> PRs are emails to your team and to your future self. Indeed! I've found many point on this discussion answered by the linux kernel idea of mailing lists where a change is discussed then approved, often with feedback acknowledged