apache
Formatting the entire code base
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
No.
This needs proper discussion in the Jackrabbit PMC and likely a vote because it'll be controversial.
I put this as a draft just to test it and see how the PR would look like :)
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.
@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.
That's why we can put the formatting commits inside .git-blame-ignore-revs
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)
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???
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.
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?
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?
Then we backport the code with the new formatting applied? The formatting doesn't change the functionality of the code.
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.
Then we backport the code with the new formatting applied?
Do you expect cherry-pick to run smoothly in this situation?
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?
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.
We regularly backport things, and the amount of work when cherry-picking is impossible is dramatically higher.
Maybe it's better to move the discussion to the mailing list: https://lists.apache.org/thread/0519b0m34m8853zxncmt4cc6omo155k4
Because 1.22 is our maintenance branch.
Anyway, I don't think we're having a productive discussion here.
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.
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.
Final remark here: It's not just Whitespace/EOL if the Javadoc gets reformatted.
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.
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.
@thomasmueller, what I wanted to point out is that even if the blame view looks fine, there is much more to it than that.
yeah not fully yet, will slowly introduce it to some part of the code base.
@steffenvan , yes.... but this PR should be closed as in the current form it's not accepted. just create dedicated PR for the indexing part as discussed.
