Formatting ratchet

[nedtwigg]

spotless {
  ratchetFrom 'origin/master'
  ...

In this mode, spotless will filter the target further, to only apply to files which have changed since origin/master. This is similar to #178 (only apply to staged files), but imo strictly better.

Lots of people are reticent to the "mega-format-commit" that happens when you go from human-formatted to machine-formatted, and this would solve this issue. It also helps to keep copyright headers up-to-date (ours aren't). Best of all, it doesn't rely on you to run the formatter before you commit. If you commit without formatting, Spotless will still remind you in the PR, ensuring that there's no need for humans to argue about formatting.

[t-rad679]

+1

Just to clarify:

• Where does the name ratchetFrom come from? The use of the term "ratchet" leads me to believe it's likely it does something I might not necessarily expect.
• What are all the possibilities of what can be specified in the ratchetFrom field? i.e., remote branches, local branches, commits, etc.

I also think it makes sense to be able to specify target and source branches (including but not limited to working copy). I'm guessing ratchetFrom specifies target?

[t-rad679]

Oh one more thing: I think it would be ideal to be able to have separate tasks for "all" vs "changed files only". This is not critical, but it seems like it would be nice.

[nedtwigg]

Where does the name ratchetFrom come from

If something "ratchets", then it can get tighter but not looser. For example, let's say you use something like SpotBugs for the first time on your project. You'll get 1,000 errors, and it will take a long time to fix them. Rather than fixing them all right now, you can say "our CI policy is to create no new errors, and it's okay if you remove old errors" aka "tighter but not looser" aka "ratchet".

Spotless uses the name target to specify "a group of files to format". ratchetFrom says "within that group of files (aka target), only apply to files which have changed since origin/master. Once every file has been changed, then every file will be formatted, and you could turn off ratchetFrom and it wouldn't make any difference. Even still, you might want to leave ratchetFrom on, because then you can change your formatting rules without forcing every target file to change all at once.

What are all the possibilities of what can be specified in the ratchetFrom field?

https://javadoc.io/static/org.eclipse.jgit/org.eclipse.jgit/5.6.1.202002131546-r/org/eclipse/jgit/lib/Repository.html#resolve-java.lang.String-

it would be ideal to be able to have separate tasks for "all" vs "changed files only"

Based on our previous conversations, I think we have irreconcilable differences around "convention-over-configuration" and default behaviors :) If people want an escape hatch, we are always okay with merging PRs which are cleanly implemented, tested, and documented. We have now merged a few such escape hatches, and I think people eventually see why the defaults are what they are, but it is easier to merge a well-tested PR than to persuade people to change what they want to do.

I also think it makes sense to be able to specify target and source branches (including but not limited to working copy). I'm guessing ratchetFrom specifies target?

I don't understand either of these sentences, probably because target means "a group of files to format" to me, and you're using it to mean something else. If the paragraphs above didn't answer your question, try to write documentation for the flags you'd like to see - that would help me understand what you want.

[t-rad679]

If something "ratchets", then it can get tighter but not looser. For example, let's say you use something like SpotBugs for the first time on your project. You'll get 1,000 errors, and it will take a long time to fix them. Rather than fixing them all right now, you can say "our CI policy is to create no new errors, and it's okay if you remove old errors" aka "tighter but not looser" aka "ratchet".

Thanks for the explanation! That's clever naming and I think it does do what I would expect.

Based on our previous conversations, I think we have irreconcilable differences around "convention-over-configuration" and default behaviors :)

Fair enough :).

I don't understand either of these sentences, probably because target means "a group of files to format" to me, and you're using it to mean something else. If the paragraphs above didn't answer your question, try to write documentation for the flags you'd like to see - that would help me understand what you want.

By source and target, I mean the source and target branches of a merge...i.e., the source would be a feature branch or a working copy and the target is typically the default branch (usually master). In other words, the files I would want to run Spotless on in this more general case could be gotten with the following command:

git diff --name-status --diff-filter=dr <target> <source>

This is what I've been trying to use in the absence of the feature this ticket is about.

The feature I'm describing would be more versatile, but, after thinking about it some more, I don't really think what I want to do is Spotless's job. I think there is a way one could use git with Spotless to make this happen (once I can get -PspotlessFiles to work properly) and that honestly seems more appropriate, though I'm not sure I can articulate why my mind changed. I could still probably be convinced back

[nedtwigg]

In your example, you are performing a diff from <target> to <source>, but you want to apply spotless to your working tree (the only place Spotless can ever work on), and there are three independent file trees: <target>, <source>, and <working tree>.

In my head, the working tree==<source>. What is the use-case where the "source" is something different than the "working tree", but it is somehow still useful for Spotless to look at files in the working tree? There could easily be files which are changed between <target> and<source> but don't exist at all in the working tree, unless you set working tree=<source>, which is the premise for ratchetFrom.

I'm curious to know about a situation where that is not true.

[sp00ne]

Is there an option to do this from HEAD instead? Say you are working on a feature branch, do atomic commits and have hooked Spotless to a pre-commit hook. There is no need to run it on already commit files imo.

Also, now spotless is aware of branching strategies on the VCS which it shouldn't. What if you refactor your strategies and change branch of "develop" or "master" into something else? It would be super hard to think of "Oh I need to change my ratchetFrom for spotless too then". It would be lost knowledge

[nedtwigg]

Yes, you can do ratchetFrom ‘HEAD’. The main downside is that it effectively turns CI format enforcement off. That’s not so bad if format enforcement is handled in your commit hooks and you trust that your contributors are using those hooks correctly.