You can cleanup your commits with a git rebase -i HEAD~<NUMBER-OF-COMMITTS>
Squash them accordingly... etc... it's an easy operation. Complementing what I said earlier... Many committs are ok.. as long as they are semantically correct... Intermediate committs that you did along your day should be cleaned up and squashed. that makes reviewing a lot easier. Even if you are committing directly upstream (you still have that right as a committer) I would ask you to review your committs before pushing. We like to have the github PRs as a peer review tool.. and quite honestly it has stopped even myself from doing damage before. I like PRs just because of that. On Wed, Apr 18, 2018 at 3:54 PM, Clebert Suconic <[email protected]> wrote: > I would prefer a squash before merging.. or at least some cleanup > before people submit a PR. > > When reviewing a PR.. it's almost impossible to make a correlation > between the committs... > > > example commit1: added a line, commit 2: removed that same line... it > would make a PR review useless and more hard. > > > I'm trying to make the review of PRs easier.. having a full day of > work being sent on a PR as a stream will make the reviewer go through > what you tried along the day. > > > After all. you should be able to checkout and compile any PR. > > > > > Anyways: I'm not asking to add the test separated as a strict rule. if > for some reason it became difficult to do it along your PR.. that's > fine. (which this will of course only apply for bug fixes). > > > > > > On Wed, Apr 18, 2018 at 3:34 PM, Michael André Pearce > <[email protected]> wrote: >> The only comment here is we will need to stop squashing our commits in >> general eg be happy a pr may have many commits I’m happy to do this btw, eg >> internally in our org we don’t care in fact we prefer the full devs commits >> as then it helps unpick stuff, I found it a shock to be squashing so much >> when I started. >> >> but generally when locally working I may commit multiple times a day >> different bits eg rework the fix or enhance the test further, it’s one of >> the big benefits of git for me, but before push and PR I squash the commits >> as up till now as we don’t seem to like to have this history. >> >> >> >> Sent from my iPhone >> >>> On 18 Apr 2018, at 18:27, Clebert Suconic <[email protected]> wrote: >>> >>> I would appreciate if we separated fixes and tests on Pull Requests. >>> >>> >>> A lot of times i will revert the fixes to validate if the test is good >>> (if it fails without a fix) and how it failed. (not that I don't trust >>> the committer, just part of the validation as sometimes I want to see >>> what was the semantic change and fix). I may eventually play a better >>> fix in the process.. and I am sure that would apply to anyone else >>> helping on reviewing commits. >>> >>> >>> I had at some point gone back in history and needed to apply the test >>> without a fix to find a better fix. >>> >>> I know eventually it's not possible to separate these.. but if you >>> could as much as possible separate them:? >>> >>> >>> I recently did that into PR #2004... >>> >>> https://github.com/apache/activemq-artemis/commit/a72046a0e32fd47cad988a8d71512927f74c8585 >>> >>> https://github.com/apache/activemq-artemis/commit/a72046a0e32fd47cad988a8d71512927f74c8585 >>> >>> >>> >>> >>> I may update the hacking guide with this.. WDYT? >>> >>> >>> -- >>> Clebert Suconic > > > > -- > Clebert Suconic -- Clebert Suconic
