First level review, #2 of 2.

Functional review:
TBD

Code review:

The git history is a little messy. Of the 6 commits on bitbucket, 4 have the same commit message, and 1 is "Manual Merge file with online IGBF-1140 branch" which sounds like it shouldn't need to be there at all. In the first commit (https://bitbucket.org/ashwiniK27/integrated-genome-browser/commits/3071c7a00fdb77274537911187f5942947fbc778?at=IGBF-1140) nbactions.xml is deleted, (this file is part of the project, it should not be deleted).

Ashwini and I worked together to start cleaning this up.

Needs just a little more git cleanup....sorry.

We don't want the history of nbactions.xml to show a commit where it was deleted and then another commit where it was added back.

use $git rebase --interactive
Set the terms to "edit" for the commit where the file was deleted (commit 68d2c89), and skip commit f7245b3 (where it was added back) by removing that line of text.
This should stop you at the commit where nbactions was deleted in the first place.
Add the file back as part of the same commit.
I think you can use $git checkout 88ce977aec49e1c7ccdd7cf10487fcd203950c1f nbactions.xml
That will pull the copy of nbactions.xml from the commit just before it was deleted.
Use $git commit --amend --no-edit
That will make adding the file back part of the same commit as where it was deleted, negating the deletion.
Then continue with the rebase, $git rebase --continue

After you push this to your remote ($git push -f), you should be able to look at the branch on bitbucket, and look at the first commit (68d2c89). We should NOT see nbactions.xml as one of the files changed.

---This worked as expected. If we ever have a commit that deletes a file (like this), I think this is the ideal way to get it back. It leaves the git history free of needless do-undo records.

With the addition of DirectoryChooserUtil.java, there are two very similar classes:
/Users/lorainelab/Documents/Repos/integrated-genome-browser/core/igb-javafx-util/src/main/java/org/lorainelab/igb/javafx/DirectoryChooserUtil.java
and
/Users/lorainelab/Documents/Repos/integrated-genome-browser/core/igb-javafx-util/src/main/java/org/lorainelab/igb/javafx/FileChooserUtil.java

Why do we need both?
I think you need to add some comments at the top of the new class explaining why we need this class. Why is FileChooserUtil.java not enough by itself?

They seem to have very similar functions.... It looks like the only difference between them is that the new class does not have all of the members that the original one has (it looses "private Optional<String> defaultFileName;" and "private Optional<List<ExtensionFilter>> extensionFilters;") and it has two functions that the original does not have.

Why not add the new functions [retrieveDirFromFxChooser() and getDirChooser()] as additional functions in the FileChooserUtil class? That seems simpler and does not produce any duplication. If the difference between these two classes is more significant than what I just described, please add comments to make that difference clear.

I'm reassigning to Ashwini to address this, and moving back to to-do.

Ivory,

You have described the difference between two files correctly. However, as FileChooserUtil.java is written to pick file from file system, I though it would be a good design to create new file for similar functionality to emphasize the difference between their objectives.
I could write both new functions in FIleChosserUtil.java as you suggested but I strongly favor adding new file instead.

Please let me know your views on this. I will change my design accordingly.

After testing this story I can confirm that the Choose Local Folder file chooser within the Data Sources tab of IGB Preferences has been changed to the operating system's native file chooser. This has been tested on Mac and Windows and is functioning as expected. Data Sources are being opened properly in IGB using the feature and there seem to be no side effects resulting from this change. Since this issue is resolved it will now be closed.