Apply spotless on whole project and get rid of ratchet

[jmazanec15]

Currently, we only apply spotless check on files that are touched only by the PR. The goal of this was to maintain git history.

However, I think we should just make one large spotless apply commit (as suggested here #328 (comment)) so that PRs dont contain changes unrelated to the PRs.

What are your thoughts @VijayanB @martin-gaievski ?

[martin-gaievski]

I agree with one time change.
How about the process after that bulky change? I vote for applying - "you touched it you own it" approach, so whatever code affected by change in PR must be addressed by author either in that PR (preferable way) or in second PR coming soon after (if bundling both changes are making PR to big/complex for review or anything like that). I think it should work with or without the "ratchet from"

[jmazanec15]

@martin-gaievski I somewhat agree with the touched it you own it approach. However, if something is pointed out that would take weeks to fix, I dont think it is the person who submitted the PRs responsibility for fixing it. I think the person should be responsible for creating the issue to track it and then it should be assigned from there. Also, it is harder to enforce this across multiple PRs with external contributors.

If they are small changes that are close (i.e. same method, pulling out duplicate code into helper methods) to the changes of the code, I think it is reasonable to fix it within that PR.

Note Once the big spotless PR is applied, ratchet from will not be needed because all files will be compliant.

[martin-gaievski]

@martin-gaievski I somewhat agree with the touched it you own it approach. However, if something is pointed out that would take weeks to fix, I dont think it is the person who submitted the PRs responsibility for fixing it. I think the person should be responsible for creating the issue to track it and then it should be assigned from there. Also, it is harder to enforce this across multiple PRs with external contributors.

If they are small changes that are close (i.e. same method, pulling out duplicate code into helper methods) to the changes of the code, I think it is reasonable to fix it within that PR.

Note Once the big spotless PR is applied, ratchet from will not be needed because all files will be compliant.

Sure, I agree that if change is serious, like for instance integration with some new version of the library it should be an issue of it's own, with the scope, estimate etc. I was more referring to smaller changes, like ones you've mentioned - pull some duplicated code to helper method or class etc. I see it as a lightweight versions of big refactoring campaigns .

[VijayanB]

IMO, every commit should leave the repository in better place than before. I will still recommend performing refactor before adding new feature if author is planning to update existing method/class, to an extent that doesn't require design change. This should be applicable to all contributors. We can be more proactive in reviewing those PR which does refactor, so that, external contributors don't feel it will take more time to clean up before adding feature.

[martin-gaievski]

I'm not sure it's always possible to foresee such refactoring tasks before doing actual change in logic/adding feature etc. It's pretty often comes up in the middle of PR or after review comments, and I was referring to that type of changes in my original comment. I agree on making refactoring beforehand if that is something we can plan