On Mon, Jul 03, 2017 at 12:14:55PM -0400, Alvaro Herrera wrote:
> Michael Paquier wrote:
> > > In passing, clean up some leftover braces which were used to create
> > > unconditional blocks. Once upon a time these were used for
> > > volatile-izing accesses to those shmem structs, which is no longer
> > > required. Many other occurrences of this pattern remain.
> >
> > Here are the places where a cleanup can happen:
> > - WalSndSetState
> > - ProcessStandbyReplyMessage
> > - XLogRead, 2 places
> > - XLogSendLogical
> > - WalRcvWaitForStartPosition
> > - WalRcvDie
> > - XLogWalRcvFlush
> > - ProcessWalSndrMessage
> > In most of the places of the WAL sender, braces could be removed to
> > improve the style. For the WAL receiver, declarations are not
> > necessary. As a matter of style, why not cleaning up just the WAL
> > sender stuff? Changing the WAL receiver code just to remove some
> > declarations would not improve readability, and would make back-patch
> > more difficult.
>
> I think we should clean this up whenever we're modifying the surrounding
> code, but otherwise we can leave well enough alone. It's not a high
> priority item at any rate.
Bundling code cleanup into commits that also do something else is strictly
worse than bundling whitespace cleanup, which is itself bad:
https://postgr.es/m/flat/20160113144826.GB3379802@tornado.leadboat.com
Deferring cleanup and pushing cleanup-only commits are each good options.