Skip to content
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

Merged
merged 1 commit into from
May 25, 2020
Merged

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented May 25, 2020

I've cherry-picked @ricardojlrufino commits to fix #10228 and:

  • added a trivial fix to function call
  • fixed indentation

Copy link
Collaborator

@matthijskooijman matthijskooijman left a 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:

  1. Refactor by renaming setLibrariesFolders to setLibrariesFoldersAndRescan, updating all call sites.
  2. Optimize by removing extra rescan calls and replace setLibrariesFoldersAndRescan with setLibrariesFolders again in the one place where the rescan happens later.

arduino-core/src/processing/app/BaseNoGui.java Outdated Show resolved Hide resolved
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
@cmaglie
Copy link
Member Author

cmaglie commented May 25, 2020

I've resolved the whitespace and squashed into a single commit.

Refactor by renaming setLibrariesFolders to setLibrariesFoldersAndRescan, updating all call sites.

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 :-)

@cmaglie cmaglie added this to the Release 1.8.13 milestone May 25, 2020
@cmaglie cmaglie merged commit b914070 into arduino:master May 25, 2020
@cmaglie cmaglie deleted the avoid-double-lib-rescan branch May 25, 2020 15:09
@ricardojlrufino
Copy link
Contributor

ricardojlrufino commented May 25, 2020

The commit history is still a bit of a mes

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)

@ricardojlrufino
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[IDE] Slow Startup - BaseNoGui.onBoardOrPortChange trigger two rescanLibraries()
4 participants