4 ms·

> i believe you've never had to go through year old large commits to figure out why a change was made? :) Frequently. Folks joke that I'm not a software engine

> i believe you've never had to go through year old large commits to figure out why a change was made? :) Frequently. Folks joke that I'm not a software engineer but a software archaeologist. As you say the line count doesn't make any difference. If the PR is doing one thing that is good. If it has a good description describing the why and linking additional tickets/documents that's even better.

nottorp 3y ago
But then you'd have to agree that as a rule of thumb 900 lines is too big.
- krisoft 3y ago
  I can only quote myself: “the line count doesn't make any difference” Just to illustrate. Imagine thet you have a bug. You trace it back either via bisect or git blame to a particular commit. Which one would you prefer: - a 5 line commit with title “Bugfixes”. 3 of those lines is changing various parameters sligtly and 2 makes some code change which seemingly “should do the same”. The associated PR has no description, and no discussion. Just an approving comment saying “LGTM”. Both the author and the approver has since left the company. Vs - a 5000 line commit with the title “Add timeout parameter to frobnicator API”, and when you check the PR in the description they explain why they think the timeout has to be configurable and link to a bug report ticket, and a design document containing lively debate about the merits of alternative solutions. There is also a comment about the testing they have done to validate the change and plots from the trial run. (Let’s also say for fairness that everyone involved in the change has since departed in this case too.) Which one do you prefer? I for sure would prefer the second one. Yes it is much much longer, but it does “one thing”, and has ample information about the context it was created in and about the thinking which was going on in the heads at the time.
  - nottorp 3y ago
    You are mixing code length with documentation length. Also there’s a bit of process fetish there

nottorp 3y ago

But then you'd have to agree that as a rule of thumb 900 lines is too big.

krisoft 3y ago
I can only quote myself: “the line count doesn't make any difference” Just to illustrate. Imagine thet you have a bug. You trace it back either via bisect or git blame to a particular commit. Which one would you prefer: - a 5 line commit with title “Bugfixes”. 3 of those lines is changing various parameters sligtly and 2 makes some code change which seemingly “should do the same”. The associated PR has no description, and no discussion. Just an approving comment saying “LGTM”. Both the author and the approver has since left the company. Vs - a 5000 line commit with the title “Add timeout parameter to frobnicator API”, and when you check the PR in the description they explain why they think the timeout has to be configurable and link to a bug report ticket, and a design document containing lively debate about the merits of alternative solutions. There is also a comment about the testing they have done to validate the change and plots from the trial run. (Let’s also say for fairness that everyone involved in the change has since departed in this case too.) Which one do you prefer? I for sure would prefer the second one. Yes it is much much longer, but it does “one thing”, and has ample information about the context it was created in and about the thinking which was going on in the heads at the time.
- nottorp 3y ago
  You are mixing code length with documentation length. Also there’s a bit of process fetish there