jhutchison edited a comment on pull request #5064:
URL: https://github.com/apache/geode/pull/5064#issuecomment-624913232
> What are the renames? I could not spot any of them with all the
reformatting.
>
> I would not reject new code formatted like this but I'd prefer that we
don't go around to all our existing code and reformat it like this. I
understand this is a stylistic issue.
> With so much whitespace and short lines it leads to not being able to have
very many statements on a screen for one viewing. I don't understand the
rational. Like it looks like variables can no longer be assigned on the same
line. For example "bossGroup =" is not longer initialized on the same line with
the constructor call (new NioEvent...). But it is okay to do this "bossGroup =
null" on one line. If this kind of reformatting is valuable then it seems like
we should have it in our coding standard. Without it being enforced I'm not
sure what will cause it to persist.
>
> One of the problems with reformatting existing code is can can cause
spurious conflicts with others working on the code. I saw this just yesterday
on the synchronized-deltas branch.
For sure stylistic- I think I'm in the minority here, so I'm ok rejecting
it if folks don't like it. For me it's much easier to look quickly and
understand that an assignment is happening when it's broken up like that. In
my mind, when an assignment is super short like x = 1, then it's really easy to
take in the totality of what is happening on the line. when it's x =
p.fill("foo").get(3).whatever(list<<object>>.class)- or something along those
lines- it takes more time to process and even see quickly where the assignment
is happening. In a similar way I like to break chained calls up on lines
because It find it much easier to quickly process the individual actions in a
chain of events that way. But again, this is me. I'm fine not merging the
commit if folks are not of the same mind as me.
The name changes are almost entirely thing like changing b into
bootStrapServer and ch into Channel- that kind of thing.
Also- again-I think I'm in the minority, but I 'm a big proponent of have
the code be narrow enough that I can see everything that's going on if I have
split window in intellij and it's still not full-screened, but again, I don't
need to push my particularities onto the team...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
