10 ms·

One feature I would’ve given a lot for when recently working with a large team on a big project: Sometimes, it’s not obvious that a thing that’s made it to the

by jrmg 4y ago

One feature I would’ve given a lot for when recently working with a large team on a big project: Sometimes, it’s not obvious that a thing that’s made it to the main branch (whether committed or merged) is problematic until a while afterwards. In that case, later commits/merges may have been made that mean the problematic one cannot be cleanly reverted (conflicts, whether by accident or because it’s been deliberately built upon). What I want to be able to do is to revert a commit _and any later commit that depends on it_ - or at least be able to see a graph that shows the dependencies to make a risk/reward trade off between reverting it all and trying to fix the problem manually. This is definitely something it’s possible to do - I’ve done it by hand too many times!

WastingMyTime89 4y ago
You are describing "rebase -i". Just don’t pick the commit you want to revert and Git will replay the following ones one by one stopping when there is an issue.
- jrmg 4y ago
  That’s a whole extra layer of tedium above what I’d like to see. I just want a single command that will do all the reverts cleanly, and the option to list the necessary reverts instead of doing them.
  - WastingMyTime89 4y ago
    > I just want a single command that will do all the reverts cleanly Well, "rebase -i" will do that if it can be done but that’s unlikely unless your commits have nothing in common. As changes will be necessary to adapt the code to the removal, it’s kind of hard to tell from the start what will be affected. Things might have knock on effects. It’s funny because I don’t really like Git but this is one of the case where what it does seem to be the best doable considering the actual complexity of the issue. If you just want to see what will be reverted you can do a three way diff between HEAD, the commit to revert and the commit you intend to revert to btw.
    - jrmg 4y ago
      I think you’re maybe not getting what I want - unless I’m misunderstanding, the three way diff you suggest won’t work for this. In the simplest possible configuration that exhibits the pattern, there are commits like this … - A - B - C. To revert A cleanly, C must first be reverted because it changes code also changed in A. B touches a different part of the code and so doesn’t need to be reverted. Factor in a few days worth of commits from a large team, and it’s easy to see how very complex trees (even graphs, I think, so long as all the dependencies point backwards) can appear. But it _is_ always possible to get a minimal ordered chain of clean reverts that are required to revert A and all its dependencies. It’s definitely true that, as you say, things might have knock-on effects - like, perhaps there no textual dependency from B to A, but the logic is very much dependent. Thorough testing of the results is needed. But in a large well-factored project the chances of things being fine when an entire dependency chain is reverted are (in my experience doing things like this) surprisingly high.
      - WastingMyTime89 4y ago
        > I think you’re maybe not getting what I want - unless I’m misunderstanding, the three way diff you suggest won’t work for this. To revert A you want to compare the difference between C and Z (commit before A) to the difference between A and Z so as to see where things which were changed in A will impact what’s in C while reverting so you can see what’s salvageable and what’s not. To come back to your chain of commits, use "rebase -i", do not pick A, and "rebase —skip" every following conflicts and you will end up in the state you want.
  - kaba0 4y ago
    Even in principle you will need 2 commit hashes to make this work, showing an editor with n lines and moving a single line up is imo easier than copy-pasting two long hashes.
unlikelytomato 4y ago
If someone has a clean commit that is a district unit of functionally and passes all tests, why would that not have simply been committed as an independent change prior to continuing work? The only way a consistent unit of code would exist to revert is by stacking it on such a change. I am not sure I have ever encountered a situation where targeting a single commit to revert would have actually worked(from a rebased merge). I would go as far as saying that git revert should be a nuclear option reserved for only the worst of situations(urgent PE hotfix?). If something is worth reverting and was missed by the engineer and the reviewers, it seems the like fix should also atomically contain the explaination with a test to prove the issue was fixed. Because of this mentality, I believe it has been several years since I have seen a 'revert' in the wild. Maybe I am alone in the industry with my opinion. However, I have seen the same patterns unfold on many teams. ...A lot of concerns with switching to a dead simple git workflow that fade almost immediately after trying it. Just my experience (backend batch jobs, applications, and rest services).
- jrmg 4y ago
  If someone has a clean commit that is a district unit of functionally and passes all tests, why would that not have simply been committed as an independent change prior to continuing work? This is exactly the kind of change I’m talking about - the problem is it’s later discovered to have flaws that the tests did not reveal, but enough time has passed that later changes depend on it. I am not sure I have ever encountered a situation where targeting a single commit to revert would have actually worked(from a rebased merge). It almost always worked in my experience. The systems we’ve worked on must be architected differently. If something is worth reverting and was missed by the engineer and the reviewers, it seems the like fix should also atomically contain the explaination with a test to prove the issue was fixed. Agreed - the problem comes if the fix is not obvious and a fix cannot immediately be applied. In our workflow, when we have belatedly learned of a problem a change is causing, a revert should then be done to keep the integrity of the main branch. I should stress that this is an exceptional case - but we did see it a few times a year.

by jrmg 4y ago

WastingMyTime89 4y ago

You are describing "rebase -i". Just don’t pick the commit you want to revert and Git will replay the following ones one by one stopping when there is an issue.

jrmg 4y ago
That’s a whole extra layer of tedium above what I’d like to see. I just want a single command that will do all the reverts cleanly, and the option to list the necessary reverts instead of doing them.
- WastingMyTime89 4y ago
  > I just want a single command that will do all the reverts cleanly Well, "rebase -i" will do that if it can be done but that’s unlikely unless your commits have nothing in common. As changes will be necessary to adapt the code to the removal, it’s kind of hard to tell from the start what will be affected. Things might have knock on effects. It’s funny because I don’t really like Git but this is one of the case where what it does seem to be the best doable considering the actual complexity of the issue. If you just want to see what will be reverted you can do a three way diff between HEAD, the commit to revert and the commit you intend to revert to btw.
  - jrmg 4y ago
    I think you’re maybe not getting what I want - unless I’m misunderstanding, the three way diff you suggest won’t work for this. In the simplest possible configuration that exhibits the pattern, there are commits like this … - A - B - C. To revert A cleanly, C must first be reverted because it changes code also changed in A. B touches a different part of the code and so doesn’t need to be reverted. Factor in a few days worth of commits from a large team, and it’s easy to see how very complex trees (even graphs, I think, so long as all the dependencies point backwards) can appear. But it _is_ always possible to get a minimal ordered chain of clean reverts that are required to revert A and all its dependencies. It’s definitely true that, as you say, things might have knock-on effects - like, perhaps there no textual dependency from B to A, but the logic is very much dependent. Thorough testing of the results is needed. But in a large well-factored project the chances of things being fine when an entire dependency chain is reverted are (in my experience doing things like this) surprisingly high.
    - WastingMyTime89 4y ago
      > I think you’re maybe not getting what I want - unless I’m misunderstanding, the three way diff you suggest won’t work for this. To revert A you want to compare the difference between C and Z (commit before A) to the difference between A and Z so as to see where things which were changed in A will impact what’s in C while reverting so you can see what’s salvageable and what’s not. To come back to your chain of commits, use "rebase -i", do not pick A, and "rebase —skip" every following conflicts and you will end up in the state you want.
- kaba0 4y ago
  Even in principle you will need 2 commit hashes to make this work, showing an editor with n lines and moving a single line up is imo easier than copy-pasting two long hashes.

unlikelytomato 4y ago

If someone has a clean commit that is a district unit of functionally and passes all tests, why would that not have simply been committed as an independent change prior to continuing work? The only way a consistent unit of code would exist to revert is by stacking it on such a change. I am not sure I have ever encountered a situation where targeting a single commit to revert would have actually worked(from a rebased merge). I would go as far as saying that git revert should be a nuclear option reserved for only the worst of situations(urgent PE hotfix?). If something is worth reverting and was missed by the engineer and the reviewers, it seems the like fix should also atomically contain the explaination with a test to prove the issue was fixed. Because of this mentality, I believe it has been several years since I have seen a 'revert' in the wild. Maybe I am alone in the industry with my opinion. However, I have seen the same patterns unfold on many teams. ...A lot of concerns with switching to a dead simple git workflow that fade almost immediately after trying it. Just my experience (backend batch jobs, applications, and rest services).

jrmg 4y ago
If someone has a clean commit that is a district unit of functionally and passes all tests, why would that not have simply been committed as an independent change prior to continuing work? This is exactly the kind of change I’m talking about - the problem is it’s later discovered to have flaws that the tests did not reveal, but enough time has passed that later changes depend on it. I am not sure I have ever encountered a situation where targeting a single commit to revert would have actually worked(from a rebased merge). It almost always worked in my experience. The systems we’ve worked on must be architected differently. If something is worth reverting and was missed by the engineer and the reviewers, it seems the like fix should also atomically contain the explaination with a test to prove the issue was fixed. Agreed - the problem comes if the fix is not obvious and a fix cannot immediately be applied. In our workflow, when we have belatedly learned of a problem a change is causing, a revert should then be done to keep the integrity of the main branch. I should stress that this is an exceptional case - but we did see it a few times a year.