12 ms·

> When was the last time you made a stcak of changes that was over 30 branches tall? Is this a thing? I think the highest stack I've ever had was 2. I suppose

> When was the last time you made a stcak of changes that was over 30 branches tall? Is this a thing? I think the highest stack I've ever had was 2. I suppose it could depend on company culture/tech stack/code organization (monorepo vs. services). > 2000-4000ln pr at least once (which could have been 30 100ln PRs). I can't say I have. Maybe deleting a lot of code where it's very obvious what is being deleted and why. What sort of work takes 2000 lines that can't be broken down into smaller, incremental pieces?

SOLAR_FIELDS 5y ago
I’m working on having a large scale open source project support a new SQL database (Postgres) and the migration work is about 2500 lines currently since I have to touch basically every part of the project. I don’t want to do the migration piecemeal since turning on the Postgres unit tests for only sections of the code base and keeping track of that and worrying about all the dependencies of code changing as other people touch the project is a lot more overhead than doing it all waterfall style. Now, I will say that these type of things are not everyday kind of PR’s, but they have certainly happened several times over the course of my career. Probably once or twice a year if I had to take a rough guess.
skrtskrt 5y ago
You can break down a big feature into smaller bits of code, but what's the point of releasing a some new code that's not being called by anything yet, when you might change it as you converge on the final solution? What's the point of a PR that's just an HTTP handler + adapter that's not exposed via an endpoint yet, and might change once we implement related the domain layer methods? What kind of context will a reviewer have on whether this is a decent handler + adapter when there is no other context about which methods the handler will call and what data will be returned? If we release the domain layer first, we may want to adjust the domain data model in order to better serve the application layer, or work better with the infrastructure/datastore layer. And so on. IMO the PRs need to be big enough to encompass an atomic, complete unit of functionality or else it's basically impossible to tell if the approach and code is any good. "It doesn't break anything" is not good enough, you can still create tech debt by releasing code that may need to be changed to complete the feature. Or someone will just implement the rest of the feature in a worse way in order to integrate with the non-ideal code that you already released when you didn't have a good enough view of the final solution
- notpachet 5y ago
  It doesn't have to be an either/or. The method that I recommend to my reports is to develop the feature in a large WIP PR branch, get some rough reviews on the macro architectural choices in that PR, and then once you have a greenlight on the larger approach, break it out into smaller PR's that each represent a smaller change necessary to effect the feature. There are definite benefits to shipping less code to production in one atomic deploy. When something breaks (and it will), you want to wade through as little code as possible while things are burning in order to identify what went wrong. Sure, you could just revert everything that went out with the last deploy, but sometimes things aren't that straightforward. Say when there are other sibling changes that are not easily roll-backable that you'd have to clobber to do a pure rollback. In these scenarios, it may be safer (albeit slower) to revert your individual change and deploy the revert by itself.
  - skrtskrt 5y ago
    I get what you mean, but rolling out in chunks can make it even harder to pinpoint and revert issues. If you roll out chunks A, B, then C, and things broke after C, the issue can still be in A, it just doesn't get exposed until it is accessed in a certain way by C. How quickly can you determine which one(s) to revert?
    - joshuamorton 5y ago
      You revert C, because that's what caused the issue. Then you investigate.
      - sciurus 5y ago
        Better yet when possible, you turn off the feature flag that enabled the code path.
- pas 5y ago
  The point is that others see that it's there, they can start to take it into account when they develop their solutions. Of course if there are problems with people depending on not yet final code that seems more of a higher level communication issue than git workflow issue. Again, if atomic unit of functionality needs huge towers of stacked PRs that indicates that they are not really atomic and the design phase (communication) seems to be missing. After all the reviewer should know about the design, the HTTP handler + adapter should not be a mystery to the reviewer. And so on... ... Now, that said, sometimes looking at a big PR as whole takes up less review time than in daily/weekly increments. But if the reviewer only sees the thing late in the dev process plus the design is not agreed upon then that means the developer writes a lot of code that might go straight to /dev/null when the reviewer disagrees with the fundamental design of it. The assumptions needed for these huge dangling WIP PRs to make sense seem very strange (risky and suboptimal) :o

SOLAR_FIELDS 5y ago

I’m working on having a large scale open source project support a new SQL database (Postgres) and the migration work is about 2500 lines currently since I have to touch basically every part of the project. I don’t want to do the migration piecemeal since turning on the Postgres unit tests for only sections of the code base and keeping track of that and worrying about all the dependencies of code changing as other people touch the project is a lot more overhead than doing it all waterfall style. Now, I will say that these type of things are not everyday kind of PR’s, but they have certainly happened several times over the course of my career. Probably once or twice a year if I had to take a rough guess.

skrtskrt 5y ago

You can break down a big feature into smaller bits of code, but what's the point of releasing a some new code that's not being called by anything yet, when you might change it as you converge on the final solution? What's the point of a PR that's just an HTTP handler + adapter that's not exposed via an endpoint yet, and might change once we implement related the domain layer methods? What kind of context will a reviewer have on whether this is a decent handler + adapter when there is no other context about which methods the handler will call and what data will be returned? If we release the domain layer first, we may want to adjust the domain data model in order to better serve the application layer, or work better with the infrastructure/datastore layer. And so on. IMO the PRs need to be big enough to encompass an atomic, complete unit of functionality or else it's basically impossible to tell if the approach and code is any good. "It doesn't break anything" is not good enough, you can still create tech debt by releasing code that may need to be changed to complete the feature. Or someone will just implement the rest of the feature in a worse way in order to integrate with the non-ideal code that you already released when you didn't have a good enough view of the final solution

notpachet 5y ago
It doesn't have to be an either/or. The method that I recommend to my reports is to develop the feature in a large WIP PR branch, get some rough reviews on the macro architectural choices in that PR, and then once you have a greenlight on the larger approach, break it out into smaller PR's that each represent a smaller change necessary to effect the feature. There are definite benefits to shipping less code to production in one atomic deploy. When something breaks (and it will), you want to wade through as little code as possible while things are burning in order to identify what went wrong. Sure, you could just revert everything that went out with the last deploy, but sometimes things aren't that straightforward. Say when there are other sibling changes that are not easily roll-backable that you'd have to clobber to do a pure rollback. In these scenarios, it may be safer (albeit slower) to revert your individual change and deploy the revert by itself.
- skrtskrt 5y ago
  I get what you mean, but rolling out in chunks can make it even harder to pinpoint and revert issues. If you roll out chunks A, B, then C, and things broke after C, the issue can still be in A, it just doesn't get exposed until it is accessed in a certain way by C. How quickly can you determine which one(s) to revert?
  - joshuamorton 5y ago
    You revert C, because that's what caused the issue. Then you investigate.
    - sciurus 5y ago
      Better yet when possible, you turn off the feature flag that enabled the code path.
pas 5y ago
The point is that others see that it's there, they can start to take it into account when they develop their solutions. Of course if there are problems with people depending on not yet final code that seems more of a higher level communication issue than git workflow issue. Again, if atomic unit of functionality needs huge towers of stacked PRs that indicates that they are not really atomic and the design phase (communication) seems to be missing. After all the reviewer should know about the design, the HTTP handler + adapter should not be a mystery to the reviewer. And so on... ... Now, that said, sometimes looking at a big PR as whole takes up less review time than in daily/weekly increments. But if the reviewer only sees the thing late in the dev process plus the design is not agreed upon then that means the developer writes a lot of code that might go straight to /dev/null when the reviewer disagrees with the fundamental design of it. The assumptions needed for these huge dangling WIP PRs to make sense seem very strange (risky and suboptimal) :o