| ▲ | throwaw12 2 hours ago | |
I am saying PRs I get are around 60-70 lines of change, which is small enough to be considered as single unit (add to this unit tests which must pass with new change, so we are talking about 30 line change + 30 line unit test) But when looking at the PR changes, you don't always see whole picture because review subjects (code lines) are scattered across files and methods, and GitHub also shows methods and files partially making it even more difficult to quickly spot the context around those updated lines. Its difficult problem, because even if GitHub shows whole body of the updated method or a file, you still don't see grand picture. For example: A (calls) -> B -> C -> D And you made changes in D, how do you know the side effect on B, what if it broke A? | ||
| ▲ | FartyMcFarter 27 minutes ago | parent | next [-] | |
If the code is well architected, the contract between C and D should make it clear whether changes in D affect C or not. And if C is not affected, then B and A won't be either. | ||
| ▲ | cesarb 29 minutes ago | parent | prev [-] | |
> Its difficult problem, because even if GitHub shows whole body of the updated method or a file, you still don't see grand picture. > For example: A (calls) -> B -> C -> D > And you made changes in D, how do you know the side effect on B, what if it broke A? That's poor encapsulation. If the changes in D respect its contract, and C respects D's contract, your changes in D shouldn't affect C, much less B or A. | ||