[trivial] fix indentation for ArgsManager class #10592

pull jnewbery wants to merge 1 commits into bitcoin:master from jnewbery:argsmanager changing 1 files +56 −55

jnewbery commented at 9:14 PM on June 14, 2017: member

Whitespace change only. Cleanup following #9494 @jtimon

Hint for reviewers: append ?w=1 to ignore whitespace: https://github.com/bitcoin/bitcoin/pull/10592/files?w=1
[trivial] fix indentation for ArgsManager class 7810993335
practicalswift commented at 10:10 PM on June 14, 2017: contributor

ACK 78109933357fdc386b75f2629d9345c215f2978f
paveljanik commented at 5:42 AM on June 15, 2017: contributor

trivial ACK 7810993
fanquake commented at 6:12 AM on June 15, 2017: member

trivialACK 7810993
ryanofsky commented at 4:55 PM on June 15, 2017: member

#10597, which is a similar whitespace-only change, was closed recently because of a developer guide admonition to "not submit patches solely to modify the style of existing code."

Is this policy worth keeping, or maybe modifying to only apply to global changes like #10597 and not local changes like this?
fanquake commented at 12:38 AM on June 16, 2017: member

@ryanofsky I think changing it to mention global vs local would be ok. Global whitespace/indentation/etc changes only serve to break pull requests and disrupt other work. Small, contained changes like this, should be fine. (and merged quickly).
theuni commented at 4:37 AM on June 16, 2017: member

@fanquake agreed, well put.
jnewbery commented at 1:48 PM on June 16, 2017: member

@ryanofsky - I think the developer guide is referring to cases when a new style is adopted by the project or becomes the norm. In those cases, contributors shouldn't open PRs to make existing code conform to the new style (as this leads to constant rebase hell).

In this case, indenting class members has always been the accepted style, and this PR just fixes the indentation that was broken by a previous PR. In fact, the author of that PR said he intended to fix this himself after merge, but that he might forget: #9494 (review).
sipa commented at 9:45 PM on June 16, 2017: member

Agree with @jnewbery; the indentation here is just broken and needs fixing. That's not the same as updating from one style to another.
jtimon commented at 5:41 PM on June 17, 2017: contributor

fast review ACK. I didn't do it at the same time for #9494 to remain easier to review.
MarcoFalke merged this on Jun 18, 2017
MarcoFalke closed this on Jun 18, 2017
MarcoFalke referenced this in commit e053e05c13 on Jun 18, 2017
PastaPastaPasta referenced this in commit 09e526d0e7 on Jul 5, 2019
PastaPastaPasta referenced this in commit 4194f07040 on Jul 5, 2019
PastaPastaPasta referenced this in commit 345d771d55 on Jul 6, 2019
PastaPastaPasta referenced this in commit 3991933b81 on Jul 8, 2019
PastaPastaPasta referenced this in commit 93683efb4e on Jul 9, 2019
PastaPastaPasta referenced this in commit e1a146d955 on Jul 9, 2019
barrystyle referenced this in commit 97ad1a1e67 on Jan 22, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

Linked (view graph)

#9494 Introduce an ArgsManager class encapsulating cs_args, mapArgs and mapMultiArgs