-
-
Notifications
You must be signed in to change notification settings - Fork 7k
Fix double libraries rescan. #10270
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
Fix double libraries rescan. #10270
Conversation
✅ Build completed. Please test this code using one of the following: ⬇️ https://downloads.arduino.cc/javaide/pull_requests/arduino-PR-10270-BUILD-971-linux32.tar.xz ℹ️ The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me, just one remark about an unneeded whitespace change.
The commit history is still a bit of a mess, with first introducing the bool option. That would be fixed by squashing into single commit by merging, but it would be even nicer to split into two commits:
- Refactor by renaming
setLibrariesFolders
tosetLibrariesFoldersAndRescan
, updating all call sites. - Optimize by removing extra
rescan
calls and replacesetLibrariesFoldersAndRescan
withsetLibrariesFolders
again in the one place where the rescan happens later.
Previously rescanLibraries() was automatically called internally in setLibrariesFolder(). This lead to double calls to rescanLibraries() when setLibrariesFolder() was used in combination with an explicit call to rescanLibraries(). This commit adds a new method setLibrariesFoldersAndRescan(..) and removes the internal call to rescanLibraries() from setLibrariesFolder(). The existing setLibrariesFolder()+rescanLibraries() combos have been replaced with setLibrariesFoldersAndRescan(). Fix arduino#10228
52c194e
to
c0315ea
Compare
I've resolved the whitespace and squashed into a single commit.
Right, that would be the best thing to do from the beginning, but doing it now it's just more work for a very little gain, so I'll leave it as is :-) |
✅ Build completed. Please test this code using one of the following: ⬇️ https://downloads.arduino.cc/javaide/pull_requests/arduino-PR-10270-BUILD-972-linux32.tar.xz ℹ️ The |
It has not yet been possible to avoid this, as there are several days of work and some corrections eventually need to be improved. End up having this, a commit, with an attempt that has not yet been resolved, but this will be discovered a little further ... (refactoring flow) |
For me the only way to solve this, was to create another branch, and reapply all modifications, try to do the isolated tests, to have a cleaner commit. But many issues are the problem of having to rewrite everything, just to have a clean commit. |
I've cherry-picked @ricardojlrufino commits to fix #10228 and: