5 ms·

> How do you tell reviewers that your reactions PR depends on the comments PR? It's at the top of pr. "someone wants to merge 1 commit into random-branch f

> How do you tell reviewers that your reactions PR depends on the comments PR? It's at the top of pr. "someone wants to merge 1 commit into random-branch from another-random-branch" > How will reviewers see the relevant diff for the reactions PR (excluding the comments PR changes)? It will show it by default? If they want to see comments PR changes, they will need to go to comments PR. > How can you propagate changes to the reactions PR if you ever need to update the comments PR before you land it (i.e. to address review comments)? Use git rebase --onto. It would be nice if the marketing page told prospective users what exact difference their tool provides - since I'm already using that workflow and never seen one of those internal tools.

tomasreimers 5y ago
Hi, sadly no marketing people at this company. Just engineers and product people. We were trying to get this to work on Github for almost a year before we decided to just build this ourselves https://twitter.com/TomasReimers/status/1325647290128850950 https://twitter.com/TomasReimers/status/1325647290128850950 Going through everything that's different is hard, but I'll try: When was the last time you made a stcak of changes that was over 30 branches tall? Why? Smaller PRs are generally agreed to be strictly better, and you've probably written a 2000-4000ln pr at least once (which could have been 30 100ln PRs). So why did you not break it out? What part of the tooling was broken?
- gkop 5y ago
  Parent did not imply that you have or should have marketing people. Your first sentence comes across as defensive. You should express thanks for this feedback and avoid sounding defensive, as that is alienating to people, stifles dialog, and casts you in a weak light. This is your moment in the HN sun, soak it up!
  - fosterfriends 5y ago
    Thank you for the feedback :)
  - tomasreimers 5y ago
    Oh hey, I'm sorry! :( That sincerely was not my intention, and a lot of context and playfulness can get lost in comments. For what it's worth, I was simply trying to make a joke about why our marketing materials may not be as good as they should be and continue the conversation. I apologize if that came off as defensive and will make sure I double check my future comments so that they don't come off in the same light.
    - CoffeeOnWrite 5y ago
      No worries! My intention was just helping you all look your best -- which you are doing with this friendly reply.
    - jacobr1 5y ago
      Ironically, this reply is also an apology! No judgment, I just found it funny.
  - kayson 5y ago
    That did not come across as defensive at all to me. It was pretty clearly tongue in cheek. Your comment, in fact, comes across as judgmental and condescending
    - CoffeeOnWrite 5y ago
      Thanks. I can see that I could have worded my feedback more tactfully, and will do so in the future.
- skipants 5y ago
  > 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
- KptMarchewa 5y ago
  I think you have an useful feature, but needing to get someone else answer me what it is in comment (https://news.ycombinator.com/item?id=29256481 https://news.ycombinator.com/item?id=29256481) isn't a good way to explain the product. Regarding your questions, the most I've done was 4, usually at most 2. My PRs are single commits that I amend and force-with-lease after getting useful code review feedback. I don't think rebasing one after another is particularly annoying, but I think not having to do it is a nice feature. I wonder whether it's a product tho. I wish you success anyway.
fosterfriends 5y ago
The docs for the CLI and dashboard go into some more depth - might be what you're looking for? - https://docs.graphite.dev/guides/graphite-cli https://docs.graphite.dev/guides/graphite-cli - https://docs.graphite.dev/guides/graphite-dashboard https://docs.graphite.dev/guides/graphite-dashboard Re `git rebase --onto`, the open source CLI offers a recursive implementation to prevent you from having to carefully rebase each branch in your stack (https://github.com/screenplaydev/graphite-cli/blob/main/src/actions/fix.ts#L179-L229 https://github.com/screenplaydev/graphite-cli/blob/main/src/...) One of the reasons you cant use a simple rebase --onto is that you dont want to accidentally copy all downstack commits between the merge base and what you're restacking onto. The CLI tracks branch bases commits through git refs to ensure that restacking never duplicates commits.
- dv_dt 5y ago
  As far as I’ve used it, git rebases ignore duplicate commits.
  - KptMarchewa 5y ago
    They are not duplicate if you change something (like, using amend) in one of the preceding branches.
    - dv_dt 5y ago
      yes that’s a possibility too, but then the typical merge resolution rules should apply. If there are common areas modified that is going to incur a manual review. There’s no magic automation I’d really entrust to fixing that.
- KptMarchewa 5y ago
  >Re `git rebase --onto`, the open source CLI offers a recursive implementation to prevent you from having to carefully rebase each branch in your stack Thanks - you've answered what I've been looking for and I'd prefer that this information was in the marketing copy. I agree that this would be an useful feature. I don't think this is a product though - I don't see myself doing more than 3 or 4 stacked diffs when this functionality would really shine.
sp332 5y ago
Merging a branch into another branch is fine, but then you have to merge that other branch up into main. And that shows effectively two branches' worth of diffs, which really muddles things if some of the changes have already been reviewed.
- tsss 5y ago
  As long as you review the 'comments' branch first, merge it, then move on to reviewing the 'reactions' branch it should be fine.
jarek83 5y ago
I second that. It's a bit surprising to me that teams could have such big PRs and branch stacks. I can only say from own perspective but I imagine this kind of nightmare would be our reality now if we didn't pay attention to improve the we work and use PRs along the way. PRs work very well for us and if we find ourselves stacking them, then we keep an eye on getting them into master (yes, we don't use the 'main' bs) in reasonable order. Then Github is actually doing job for us changing the target branch once the underlaying changes are merged into it.

tomasreimers 5y ago

Hi, sadly no marketing people at this company. Just engineers and product people. We were trying to get this to work on Github for almost a year before we decided to just build this ourselves https://twitter.com/TomasReimers/status/1325647290128850950 https://twitter.com/TomasReimers/status/1325647290128850950 Going through everything that's different is hard, but I'll try: When was the last time you made a stcak of changes that was over 30 branches tall? Why? Smaller PRs are generally agreed to be strictly better, and you've probably written a 2000-4000ln pr at least once (which could have been 30 100ln PRs). So why did you not break it out? What part of the tooling was broken?

gkop 5y ago
Parent did not imply that you have or should have marketing people. Your first sentence comes across as defensive. You should express thanks for this feedback and avoid sounding defensive, as that is alienating to people, stifles dialog, and casts you in a weak light. This is your moment in the HN sun, soak it up!
- fosterfriends 5y ago
  Thank you for the feedback :)
- tomasreimers 5y ago
  Oh hey, I'm sorry! :( That sincerely was not my intention, and a lot of context and playfulness can get lost in comments. For what it's worth, I was simply trying to make a joke about why our marketing materials may not be as good as they should be and continue the conversation. I apologize if that came off as defensive and will make sure I double check my future comments so that they don't come off in the same light.
  - CoffeeOnWrite 5y ago
    No worries! My intention was just helping you all look your best -- which you are doing with this friendly reply.
  - jacobr1 5y ago
    Ironically, this reply is also an apology! No judgment, I just found it funny.
- kayson 5y ago
  That did not come across as defensive at all to me. It was pretty clearly tongue in cheek. Your comment, in fact, comes across as judgmental and condescending
  - CoffeeOnWrite 5y ago
    Thanks. I can see that I could have worded my feedback more tactfully, and will do so in the future.
skipants 5y ago
> 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
KptMarchewa 5y ago
I think you have an useful feature, but needing to get someone else answer me what it is in comment (https://news.ycombinator.com/item?id=29256481 https://news.ycombinator.com/item?id=29256481) isn't a good way to explain the product. Regarding your questions, the most I've done was 4, usually at most 2. My PRs are single commits that I amend and force-with-lease after getting useful code review feedback. I don't think rebasing one after another is particularly annoying, but I think not having to do it is a nice feature. I wonder whether it's a product tho. I wish you success anyway.

fosterfriends 5y ago

The docs for the CLI and dashboard go into some more depth - might be what you're looking for? - https://docs.graphite.dev/guides/graphite-cli https://docs.graphite.dev/guides/graphite-cli - https://docs.graphite.dev/guides/graphite-dashboard https://docs.graphite.dev/guides/graphite-dashboard Re `git rebase --onto`, the open source CLI offers a recursive implementation to prevent you from having to carefully rebase each branch in your stack (https://github.com/screenplaydev/graphite-cli/blob/main/src/actions/fix.ts#L179-L229 https://github.com/screenplaydev/graphite-cli/blob/main/src/...) One of the reasons you cant use a simple rebase --onto is that you dont want to accidentally copy all downstack commits between the merge base and what you're restacking onto. The CLI tracks branch bases commits through git refs to ensure that restacking never duplicates commits.

dv_dt 5y ago
As far as I’ve used it, git rebases ignore duplicate commits.
- KptMarchewa 5y ago
  They are not duplicate if you change something (like, using amend) in one of the preceding branches.
  - dv_dt 5y ago
    yes that’s a possibility too, but then the typical merge resolution rules should apply. If there are common areas modified that is going to incur a manual review. There’s no magic automation I’d really entrust to fixing that.
KptMarchewa 5y ago
>Re `git rebase --onto`, the open source CLI offers a recursive implementation to prevent you from having to carefully rebase each branch in your stack Thanks - you've answered what I've been looking for and I'd prefer that this information was in the marketing copy. I agree that this would be an useful feature. I don't think this is a product though - I don't see myself doing more than 3 or 4 stacked diffs when this functionality would really shine.

sp332 5y ago

Merging a branch into another branch is fine, but then you have to merge that other branch up into main. And that shows effectively two branches' worth of diffs, which really muddles things if some of the changes have already been reviewed.

tsss 5y ago
As long as you review the 'comments' branch first, merge it, then move on to reviewing the 'reactions' branch it should be fine.

jarek83 5y ago

I second that. It's a bit surprising to me that teams could have such big PRs and branch stacks. I can only say from own perspective but I imagine this kind of nightmare would be our reality now if we didn't pay attention to improve the we work and use PRs along the way. PRs work very well for us and if we find ourselves stacking them, then we keep an eye on getting them into master (yes, we don't use the 'main' bs) in reasonable order. Then Github is actually doing job for us changing the target branch once the underlaying changes are merged into it.