Hi John, On 30/6/21 2:57 pm, John Cotton Ericson wrote:> The nature of the change is putting a decent chunk of existing > autoconf m4 within > a newly-introduced if-then-else. That means either the indention will > become wrong, > or there will be some churn/noise reindenting the old code. > > I chose to reindent first, making on odd 2x indent, and then add the > new code and > if-then-else, also fixing the indent.The problem is that a diff for a version from before your patch comparing with a version after will be noisy.? That's what I want you to avoid.> A third option would be to simply do the first of those to patches, > making the minimal > change and then leaving the indentation incorrect.That's what I think you should do.? Adding a comment at the if-then-else saying why the indent is wrong might help avoid flammage about wrongly indented code. Regards, David
Three other options: With diff use one of theses options: -E --ignore-tab-expansion, -Z --ignore-trailing-space, -b --ignore-space-change, -w --ignore-all-space, and/or -B --ignore-blank-lines With git-blame use the -w option. With git do all format changes in separate commits, then add commit(s) to .ignoreRevsFile so these format only commits do not show up in git blame. Best used when doing mass formatting changes to the source, then inforce formatting standards when doing new commits. On 6/30/2021 5:14 AM, David Newall wrote:> Hi John, > > On 30/6/21 2:57 pm, John Cotton Ericson wrote: >> The nature of the change is putting a decent chunk of existing autoconf m4 within >> a newly-introduced if-then-else. That means either the indention will become wrong, >> or there will be some churn/noise reindenting the old code. >> >> I chose to reindent first, making on odd 2x indent, and then add the new code and >> if-then-else, also fixing the indent. > > The problem is that a diff for a version from before your patch comparing with a version > after will be noisy.? That's what I want you to avoid. > >> A third option would be to simply do the first of those to patches, making the minimal >> change and then leaving the indentation incorrect. > > That's what I think you should do.? Adding a comment at the if-then-else saying why the > indent is wrong might help avoid flammage about wrongly indented code. > > Regards, > > David > _______________________________________________ > openssh-unix-dev mailing list > openssh-unix-dev at mindrot.org > https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev-- Douglas E. Engert <DEEngert at gmail.com>
Douglas, Thanks. Those are good hidden gems with git, for sure! Correct me if I am wrong, but since I am using git-send-email/git-format-patch the diff ones don't apply to me, right? Those would be for e.g. David and reviewers in general, not patch submitters? Thus, the additional options for *creating* the patches you are proposing are: * Combine patch, rely on reviewer to use the provided diff options * Split patch, create a file like https://github.com/llvm/llvm-project/blob/main/.git-blame-ignore-revs for reviewers to use with git blame. ? John On Wed, Jun 30, 2021, at 8:37 AM, Douglas E Engert wrote:> Three other options: > With diff use one of theses options: -E --ignore-tab-expansion, -Z --ignore-trailing-space, > -b --ignore-space-change, -w --ignore-all-space, and/or -B --ignore-blank-lines > > With git-blame use the -w option. > > With git do all format changes in separate commits, then add commit(s) to .ignoreRevsFile > so these format only commits do not show up in git blame. Best used when doing mass formatting > changes to the source, then inforce formatting standards when doing new commits. > > > On 6/30/2021 5:14 AM, David Newall wrote: > > Hi John, > > > > On 30/6/21 2:57 pm, John Cotton Ericson wrote: > >> The nature of the change is putting a decent chunk of existing autoconf m4 within > >> a newly-introduced if-then-else. That means either the indention will become wrong, > >> or there will be some churn/noise reindenting the old code. > >> > >> I chose to reindent first, making on odd 2x indent, and then add the new code and > >> if-then-else, also fixing the indent. > > > > The problem is that a diff for a version from before your patch comparing with a version > > after will be noisy. That's what I want you to avoid. > > > >> A third option would be to simply do the first of those to patches, making the minimal > >> change and then leaving the indentation incorrect. > > > > That's what I think you should do. Adding a comment at the if-then-else saying why the > > indent is wrong might help avoid flammage about wrongly indented code. > > > > Regards, > > > > David > > _______________________________________________ > > openssh-unix-dev mailing list > > openssh-unix-dev at mindrot.org > > https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev > > -- > > Douglas E. Engert <DEEngert at gmail.com> > > _______________________________________________ > openssh-unix-dev mailing list > openssh-unix-dev at mindrot.org > https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev >