-
Notifications
You must be signed in to change notification settings - Fork 465
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
using ratchetFrom in a branch fails to detect the previous index in origin/main #1372
Comments
I faced this today again but this time there was no branch. I did everything in the main branch, added some files, updated some files. After running |
Thanks for reporting this problem and sorry for the delay! I will investigate this week. |
Hi @bwgjoseph, I played around with The first line in the index file is fingerprint. It is based on:
The fingerprint will change if you bump Have spotless failed to format files or detect unformatted files when you switched branched? Or was |
Thanks, @lutovich for looking into it.
Should this be the case? Because by updating the config/version, it loses all the
No, or at least none that I can see. As also mentioned above, this happens even when I did not switch branch. And after your explanation, I will add that I did not make configuration change the pom section, or at least, what was configured for spotless. To further clarify, you mentioned serialized the entire |
@bwgjoseph fingerprint has to change when plugin version or formatter configuration changes. After a configuration change, the plugin no longer knows if the tracked files are correctly formatted and needs to re-process them. I think Spotless plugin configuration changes should be relatively infrequent and index invalidations should not affect performance. Fingerprint includes serialized
It is good to know that the plugin formats correctly. Do you remember if you've added/changed any non-spotless dependencies? |
So to say that the
That means if I don't have this enabled, (without having the index file), every time I run
Ah, I thought that you were saying that any changes to the entire I could try to add new dependencies/configuration to and see if the fingerprint will mismatch again. |
I can verify that the |
Would like to know if the Especially, like what is the flow for running |
Thanks for checking this. I'm not sure The index file keeps track of well-formatted files for a particular Spotless plugin configuration (represented by |
I understand that But I'm more concerned on how to use I think as you have explained before, And I still have concern when using with large codebase, if the Thanks for investigating, and answering to my queries. Appreciate it. |
Hi @bwgjoseph, Sorry for the delay! I created a PR to make the plugin reset the index only when plugin version changes or when its configuration changes: #1461. In a multi-module project setup each module has a separate index file. A version or configuration change in the parent POM will invalidate index files in all modules. I think plugin upgrades and re-configurations are relatively infrequent compared to code changes that need to run When you pull changes made by other developers, the changed files will have a different last-modified timestamp compared to your local index entries. It means Spotless will have to re-format those files. I think if you pull often, then performance impact should be limited. However, if you pull and most of the codebase has changed then Spotless will have to re-format/re-check formatting for the entire codebase. |
Thanks for the PR! I think what you described make sense, and with the PR, it should make the invalidation less frequent. I currently have a multi-module project with the following setup.
The Is that right? Also, to verify, the
In terms of PR, it is the responsibility of the developer to ensure the latest index file is also committed, is that right? |
I think it makes sense that the index file is empty in I don't think it's a good idea to commit the index file. It would be updated for every code change and will probably be a constant source of merge conflicts. By default, Spotless puts the index file in the |
Somehow, I always had the impression to commit the index file so that the formatter does not need to re-run. But I never thought the other way round that if every developer had run at least once, they would have the updated index file anyway. The reason why I wanted it to be outside the Am I not right in that? Did I misunderstand a certain concept? I don't think I have exceptionally huge repo, it will grow but still manageable size. Do you think that letting it rebuild each time Thanks! |
Sounds like a fair reason to put the index file outside of I guess it comes down to how your team runs Maven commands. If a typical command includes |
Nowadays, I don't think But I'll take your suggestion to git-ignore it. Given that the PR is merged, any chance of releasing new version soon? |
A new release will be cut in a few days, and I post the release version into all affected issues and pr's so you'll get a notification if you're subscribed to this issue. |
Published in |
Hi,
I'm using
maven (wrapper) 3.8.4
withspotless-maven-plugin 2.27.1
with the configuration defined as followsSo I have been working on
main
for a while, and it already has a.spotless-index
file with the checksum and the listing of all the files. So far, no issue with that. But recently, I created a branch off mymain
calledfeature-a
, and after the first commit infeature-a
branch, I ranspotless:apply
, then the.spotless-index
file now has a newchecksum
and only listing files that was changed infeature-a
branch.Imagine in
main
branchThen in
feature-a
branch (after a commit, and ran spotless:apply)There was a log indicating
I can't have the full logs pasted as this occurs in an air-gapped repo, so if there is further information required, please let me know.
However, based on one of my previous feature branch, it doesn't seem to suffer the same issue. So I don't quite understand if there was something that I did wrong when creating the branch, or some mis-configuration on my end.
Thanks!
The text was updated successfully, but these errors were encountered: