Formatting the entire code base

[steffenvan]

Reformatted all the code with https://google.github.io/styleguide/javaguide.html without polluting the history.

See: https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view

[reschke]

No.

This needs proper discussion in the Jackrabbit PMC and likely a vote because it'll be controversial.

[steffenvan]

I put this as a draft just to test it and see how the PR would look like :)

[steffenvan]

I can see that deciding on which formatter we should use could warrant a bit of discussion. But I find it strange that introducing one would be controversial. On the contrary, the fact that we don't have one is concerning. Especially considering how easy it is to introduce and that it won't clutter the blame view.

[mbaedke]

@steffenvan, it messes up diffs.

[steffenvan]

@mbaedke how so?

[mbaedke]

@steffenvan, If you - for example during debugging - compare between revisions to see what has changed and git will always tell you that everything did, it's very annoying.

[steffenvan]

That's why we can put the formatting commits inside .git-blame-ignore-revs

[reschke]

but that doesn't help in all cases, for instance:

• when diffing with other branches that may have not been reformatted
• when trying to cherrypick changes to maintenance branches (right?)
• when trying to understand how the codebase differs from a completely different repo that we copied (such as recently for lucene-core)

[mbaedke]

Also we are talking git diff, not git blame, right? I guess you can use git blame in an intermediate step to produce a list of files for the diff, but really???

[mbaedke]

the fact that we don't have one is concerning

May I ask what's so critical about that?
Also, nobody cares about the GitHub blame view. People want to use the command line to make their actions scriptable.

[steffenvan]

If you - for example during debugging - compare between revisions to see what has changed and git will always tell you that everything did, it's very annoying.

to me that also sounds like a one time thing. If both revisions have the same formatting, this should not be a problem?

[reschke]

If both revisions have the same formatting, this should not be a problem?

So do you propose to reformat the current maintenance branch and historic branches as well? What if we want to backport a commit to the maintenance branch?

[steffenvan]

Then we backport the code with the new formatting applied? The formatting doesn't change the functionality of the code.

[mbaedke]

to me that also sounds like a one time thing. If both revisions have the same formatting, this should not be a problem?

Again, if you want to detect what has changed at all and you compare with an old revision, it's very annoying.

[mbaedke]

Then we backport the code with the new formatting applied?

Do you expect cherry-pick to run smoothly in this situation?

[steffenvan]

Do you expect cherry-pick to run smoothly in this situation?

Depending on the amount of changes, it's going to be pretty simple to fix the conflicts yeah. Just keep the formatting in one commit and cherry picking in another.

Besides, how often do we need to backport? And how often are we maintaining, refactoring and adding new features in Oak? And more importantly, which one are we doing the most?

[steffenvan]

Again, if you want to detect what has changed at all and you compare with an old revision, it's very annoying.

Working on a codebase with inconsistent formatting is also not without friction.

[reschke]

We regularly backport things, and the amount of work when cherry-picking is impossible is dramatically higher.

[steffenvan]

Question:
Why are we regularly backporting in the first place?

[steffenvan]

Maybe it's better to move the discussion to the mailing list: https://lists.apache.org/thread/0519b0m34m8853zxncmt4cc6omo155k4

[reschke]

Because 1.22 is our maintenance branch.

Anyway, I don't think we're having a productive discussion here.

[mbaedke]

Maybe it's better to move the discussion to the mailing list

That makes sense. My Prediction: you will face a lot of resistance :)
For (at least) the reasons mentioned, committers here are very reluctant to make cosmetic changes at all.

[steffenvan]

My Prediction: you will face a lot of resistance :)

I already am ;) Doesn't change the fact that it brings huge advantages. There is a reason most organizations have a very strict guideline.

[mbaedke]

Final remark here: It's not just Whitespace/EOL if the Javadoc gets reformatted.

[thomasmueller]

nobody cares about the GitHub blame view

I do. I do a lot! That's how I find out why the code is like this. This is called "Chesterton's Fence." and it's quite an important part of what code maintenance means.

[thomasmueller]

Maybe it's better to move the discussion to the mailing list

Well let's discuss here, and then if we use the mailing list we need a link there.

[mbaedke]

@thomasmueller, what I wanted to point out is that even if the blame view looks fine, there is much more to it than that.

[reschke]

This PR changes thousands of files. This is a blocker for me.