Generally it’s more considerate to submit smaller batches of (self-contained!) work at a time. Sometimes the work or existing code is so interconnected that this can’t really be done, though. You mentioned spending a month, that’s also a long time in most environments to be off working on bulk changes for later review. A week is probably long enough in most cases, some orgs even prefer to push (if not review) code daily.
Don’t agonize over it though, wanting to improve your impact on others while working is the right direction to point, keep walking that way and you’ll do awesome, let it develop over time. As in, don’t let your desire for politeness slow down your work or growth too much (I do this lol, why I’m mentioning).
I’ll try to keep that in mind. What do I do the next time I’m working on a very long branch? The next big project to do on this thing is fixing the myriad localization issues that have been plaguing this project, and that’ll mean pruning maybe thousands of lines. Merging my fixes to the main branch was apparently a nightmare (I’ve already apologized. Profusely. And haven’t done it again.)
Should I just try to get as much work done in as few commits as possible?
First off, don’t sweat it, the “nightmare” was probably 20-30 annoying minutes for the senior.
But commit count isn’t what would’ve made it tough to merge, and strictly speaking smaller commits is typically better.
Concretely: limit the kinds and the reach of changes you’re asking someone to review & merge. I’ve never done localization but I’ll try to guess for an example -
1000 lines changed because you’re replacing an incorrect punctuation character misused everywhere - that’s really only one change to have to think about, it’s 1000 lines but one fairly trivial change.
Now, if each of those lines was improving some phrasing, done by a native speaker that found existing phrasing clunky for their language? Well, each of those are their own judgment call and have to be assessed individually. A thousand fairly subtle changes, and a PR isn’t meant to accept some and reject others.
More abstractly, try to put yourself in the reviewer’s shoes (hard when new) - how much stuff do they have to keep in mind in order to make judgment calls about this whole batch of code?
More if you want it:
spoiler
Moving on from localization, ask yourself, how many “places” in the codebase does your code touch? Are you lumping every single possible thing into a single change set because it feels like a single feature, when it could have been architecturally segmented? Like sure a single PR for a single overall feature makes sense conceptually, but can it be split? Your skill with splitting up tasks is like 70% of your skill as an engineer over time, btw.
So for example, single feature - all code and PR for DB models, separate PR for API updates, separate PR for UI updates. One feature but the UI folks don’t have to think about the other parts, etc. Maybe the ideal case is obvious.
There are always complicating factors. Some tasks aren’t well suited to that segmentation and really require big far reaching changes. Some codebases (bad ones) make it impossible to change one part without touching ~all of it.
When you’re starting out, doing this reliably well requires seniors scoping out tasks properly so that they are developed (and then reviewed) in a sensible, segmented way.
Generally it’s more considerate to submit smaller batches of (self-contained!) work at a time. Sometimes the work or existing code is so interconnected that this can’t really be done, though. You mentioned spending a month, that’s also a long time in most environments to be off working on bulk changes for later review. A week is probably long enough in most cases, some orgs even prefer to push (if not review) code daily.
Don’t agonize over it though, wanting to improve your impact on others while working is the right direction to point, keep walking that way and you’ll do awesome, let it develop over time. As in, don’t let your desire for politeness slow down your work or growth too much (I do this lol, why I’m mentioning).
I’ll try to keep that in mind. What do I do the next time I’m working on a very long branch? The next big project to do on this thing is fixing the myriad localization issues that have been plaguing this project, and that’ll mean pruning maybe thousands of lines. Merging my fixes to the main branch was apparently a nightmare (I’ve already apologized. Profusely. And haven’t done it again.)
Should I just try to get as much work done in as few commits as possible?
First off, don’t sweat it, the “nightmare” was probably 20-30 annoying minutes for the senior.
But commit count isn’t what would’ve made it tough to merge, and strictly speaking smaller commits is typically better.
Concretely: limit the kinds and the reach of changes you’re asking someone to review & merge. I’ve never done localization but I’ll try to guess for an example -
1000 lines changed because you’re replacing an incorrect punctuation character misused everywhere - that’s really only one change to have to think about, it’s 1000 lines but one fairly trivial change.
Now, if each of those lines was improving some phrasing, done by a native speaker that found existing phrasing clunky for their language? Well, each of those are their own judgment call and have to be assessed individually. A thousand fairly subtle changes, and a PR isn’t meant to accept some and reject others.
More abstractly, try to put yourself in the reviewer’s shoes (hard when new) - how much stuff do they have to keep in mind in order to make judgment calls about this whole batch of code?
More if you want it:
spoiler
Moving on from localization, ask yourself, how many “places” in the codebase does your code touch? Are you lumping every single possible thing into a single change set because it feels like a single feature, when it could have been architecturally segmented? Like sure a single PR for a single overall feature makes sense conceptually, but can it be split? Your skill with splitting up tasks is like 70% of your skill as an engineer over time, btw.
So for example, single feature - all code and PR for DB models, separate PR for API updates, separate PR for UI updates. One feature but the UI folks don’t have to think about the other parts, etc. Maybe the ideal case is obvious.
There are always complicating factors. Some tasks aren’t well suited to that segmentation and really require big far reaching changes. Some codebases (bad ones) make it impossible to change one part without touching ~all of it.
When you’re starting out, doing this reliably well requires seniors scoping out tasks properly so that they are developed (and then reviewed) in a sensible, segmented way.
This is extremely helpful, thank you so much.