Formatting improvements

[badasahog]

Partial progress toward closing #7049 and #7023

I want to make sure maintainers are on board or at least provide feedback if they thing this style is bad.

After this is done, the best way to improve readability (in my opinion) is to rename identifiers, and create well named utility functions.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.69%. Comparing base (5bbc4d5) to head (f34faae).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7052   +/-   ##
=======================================
  Coverage   98.69%   98.69%           
=======================================
  Files          79       79           
  Lines       14676    14676           
=======================================
  Hits        14485    14485           
  Misses        191      191           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

• HEAD=formattingConsistency stopped early for DT[by,verbose=TRUE] improved in #6296
• HEAD=formattingConsistency slower P<0.001 for setDT improved in #5427
  

Generated via commit f34faae

Download link for the artifact containing the test results: ↓ atime-results.zip

Task	Duration
R setup and installing dependencies	2 minutes and 49 seconds
Installing different package versions	19 seconds
Running and plotting the test cases	2 minutes and 0 seconds

[aitap]

At the very least, let's not touch the indentation for now (see assign.c).

[jangorecki]

Wasn't that meant to be for fread only at the moment?

I would appreciate at least an analysis how many conflicts it will cause to open PRs before deciding on merging that.
As well said by @aitap

high effort/reward ratio. At best the code will work as well as
it used to; more likely there will be even more problems to fix as a
result of the fix

Is it so difficult to understand that we have limited resources on pursuing some of PRs that are pending for 2-3 years, and this single PR does not resolve any problem but create a risk that those resources will run out before those pending PRs will be merged?

[badasahog]

@jangorecki I respect your feedback. I'll localize to fread instead in a separate pr, but I don't think there's any harm in keeping this open.

In terms of your assertion that big pull requests like this slow down development, I think resolving these collisions would be fairly straight forward, and will help make development smoother in the future.

Again, I can tell that this may have been over zealous, so I'll try to read the room, and stick with smaller PRs, like ones just localized to fread.

[MichaelChirico]

I think resolving these collisions would be fairly straight forward

I think in isolation, yes, but they quickly become entangled with real changes and the mental load to tease apart "real" vs superficial changes increases something like exponentially.

[TysonStanley]

I certainly don't mind the style, I'm a fan of well formatted code like this. However, I have to agree with @MichaelChirico and @jangorecki knowing how many collisions will occur with already existing PRs and other large upcoming PRs.

[jangorecki]

Here is an example of a tiny change and a conflict that it created, just from 2 weeks ago, resolved last week: 82e66e7#r158650259
It is only on one of the PRs, same and then also more complex conflicts (because other PRs modified that code again) will have to be resolved again, and again, in other PRs that are waiting in the queue.
This just doesn't make sense. If there is code formatting to be applied on a file, let's first merge (or close) all PRs that are attempting to modify that file, and then apply formatting change. As simple as that.

[MichaelChirico]

Following up once more:

• We really do appreciate such changes. It's with regrets that we don't accept them at the moment due to a slowly-clearing PR backlog (our fault, not yours). We recognize the value of code quality changes which just generally make it less tedious + reduce cognitive load to do new development/bug fixing work.
• You're absolutely right there's no harm to leaving this open, please do. Thank you for all your recent contributions!