![]() The good news is that GitHub actually supports a much better alternative which works really nicely. It breaks git bisect, git revert, and git blame.After merging the pull request, it pollutes the project’s history with useless noise: fixup commits which are not linked to the commits they are fixing.The only workaround is to review via the “Files changed” tab, but that brings considerable disadvantages when the pull request contains more than one commit, because then the reviewer cannot review individual logical changes one at a time. each time fixup commits are appended to the pull request. This problem is significantly compounded each review cycle, i.e. ![]() If it does fix it, selectively ignore that brokenness, whilst taking care not to accidentally ignore other brokenness in the same area.If such a commit is found, verify that it really does fix the brokenness.If they spot brokenness, refrain from immediately commenting on it, and instead look ahead for any subsequent commit in the series which may claim to fix that brokenness.So for each change, they have follow this process: It makes life much harder for reviewers, because they have to review a combination of broken commits and subsequent commits which are supposed to fix that brokenness. But that’s almost always a really bad idea, because again it violates the rule of one logical change per commit. However in most cases this is a bad idea, because each time you do it, you create an “orphaned” abandoned pull request, and there is no automatic linking between the new revised one and the abandoned one, which makes it a lot harder to follow the history of the review process.Īnother approach (the one apparently implied by the GitHub documentation mentioned above) is to append “fixup” commits to the series which fix issues introduced by commits earlier in the series. One approach is for the submitter fix the issues in their original local branch, close the pull request, and submit a new one.
0 Comments
Leave a Reply. |
AuthorWrite something about yourself. No need to be fancy, just an overview. ArchivesCategories |