4 ms·

How does HN deal with refactorings that litterally touch 90% of the code, but just 3 lines? Huge PR to refactor, and fatigue sets in. But we cannot PR chunks b

How does HN deal with refactorings that litterally touch 90% of the code, but just 3 lines? Huge PR to refactor, and fatigue sets in. But we cannot PR chunks because the build cannot fail.

joaoqalves 4y ago
Why can't you create PR chunks without the build failing? Usually, I strive for "vertical slices". That means that, sometimes, a PR just contains "useless" code in isolation, but that makes sense as I keep merging them (stacked diffs). My flow is usually: 1. Does this code have tests? What's the minimum chunk I need to write tests for it? 2. Write tests and the smallest refactor I can think of (it may be to create one function, p.e.) 3. Commit 4. Return to 1. until the full refactor is done 5. Throw 3-10 stacked diffs that can be merged with the build passing. EDIT: formatting
jstanley 4y ago
What does it mean for a refactor to touch 90% of the code, but just 3 lines? There's only 3.333 lines in total??
- masklinn 4y ago
  I assume the feature itself is just 3 lines, but all the changes necessary for those 3 lines to be possible to add impact the code significantly because e.g. there’s bits of data which need to be threaded through the entire thing and previously were not.
dcsommer 4y ago
Commit description should contain the command to generate the change. Your review the command plus spot check actual changes and CI signals.
codemac 4y ago
> But we cannot PR chunks because the build cannot fail Could you code review & approve on a branch without those build restrictions, and then have an additional review of a merge commit to main? The "has to build" requirement has been circumvented at all previous employers with some similar workflow, including on perforce, cvs, email, mercurial, and git.
atq2119 4y ago
Split into two commits: the first commit does a mechanical refactoring but contains no (intended) functional changes. The second commit contains the 3 lines of functional change.

How does HN deal with refactorings that litterally touch 90% of the code, but just 3 lines? Huge PR to refactor, and fatigue sets in. But we cannot PR chunks because the build cannot fail.

joaoqalves 4y ago

Why can't you create PR chunks without the build failing? Usually, I strive for "vertical slices". That means that, sometimes, a PR just contains "useless" code in isolation, but that makes sense as I keep merging them (stacked diffs). My flow is usually: 1. Does this code have tests? What's the minimum chunk I need to write tests for it? 2. Write tests and the smallest refactor I can think of (it may be to create one function, p.e.) 3. Commit 4. Return to 1. until the full refactor is done 5. Throw 3-10 stacked diffs that can be merged with the build passing. EDIT: formatting

jstanley 4y ago

What does it mean for a refactor to touch 90% of the code, but just 3 lines? There's only 3.333 lines in total??

masklinn 4y ago
I assume the feature itself is just 3 lines, but all the changes necessary for those 3 lines to be possible to add impact the code significantly because e.g. there’s bits of data which need to be threaded through the entire thing and previously were not.

dcsommer 4y ago

Commit description should contain the command to generate the change. Your review the command plus spot check actual changes and CI signals.

codemac 4y ago

> But we cannot PR chunks because the build cannot fail Could you code review & approve on a branch without those build restrictions, and then have an additional review of a merge commit to main? The "has to build" requirement has been circumvented at all previous employers with some similar workflow, including on perforce, cvs, email, mercurial, and git.

atq2119 4y ago

Split into two commits: the first commit does a mechanical refactoring but contains no (intended) functional changes. The second commit contains the 3 lines of functional change.