Skip to content

Renaming folders - #16

Closed
adamrankin wants to merge 1 commit into
masterfrom
folder_rename
Closed

adamrankin wants to merge 1 commit into
masterfrom
folder_rename

Conversation

@adamrankin

Copy link
Copy Markdown
Member

CADModels -> Models
TestImages -> Volumes

CADModels -> Models
TestImages -> Volumes
@lassoan

lassoan commented Feb 1, 2018

Copy link
Copy Markdown
Contributor

I agree that the current folder structure is not good (for example, TestImages do not contain only images but we had to add 2 STL files in TestImages to make the ultrasound simulator work), so I support the idea of reorganization.

However, this is not a trivial task. I think the proposed names are not making more clear what each folder should contain. Changing the folders require many changes in the code (see list of impacted files below), so if we change it, then we have to make sure we get it right.

Distinguishing folders based on file type only (model or image) does not seem to be useful to me. A better option could be to merge CADModels and TestImages folders into one. We could also add many subfolders for images (similarly to CADModels) to keep things that belong to each other, close to each other. Or, maybe we should just find a more descriptive name for TestImages.

What was your main motivation for the folder rename?


List of impacted files:

PlusLib\src\CMakeLists.txt
PlusLib\src\PlusDataCollection\Testing\CMakeLists.txt
PlusLib\src\Documentation\UserManual\ApplicationVolumeReconstructor.dox
PlusLib\src\PlusImageProcessing\Testing\CMakeLists.txt
PlusLib\src\PlusWidgets\Testing\CMakeLists.txt
PlusLib\src\PlusUsSimulator\Testing\CMakeLists.txt
PlusLib\src\PlusVolumeReconstruction\Testing\CMakeLists.txt
PlusLib\src\PlusOpenIGTLink\Testing\CMakeLists.txt
PlusLib\src\PlusDataCollection\Testing\vtkDataCollectorFileTest.cxx
PlusLib\src\PlusCommon\Testing\CMakeLists.txt
PlusLib\src\PlusCalibration\Testing\CMakeLists.txt
PlusLib\src\Documentation\UserManual\ApplicationExtractScanLines.dox
PlusLib\src\CMakeLists.txt

PlusApp\InstallFiles.cmake
PlusApp\fCal\Testing\CMakeLists.txt
PlusApp\SpatialSensorFusion\Testing\CMakeLists.txt
PlusApp\InstallFiles.cmake
PlusApp\Documentation\ApplicationSpatialSensorFusion.dox

@adamrankin

Copy link
Copy Markdown
Member Author

I want to download images and polydata from PlusServer to a client. TestImages did not seem like a fitting name to store images.

@adamrankin

Copy link
Copy Markdown
Member Author

The new names are more generic, and thus more flexible. The Volumes folder can contain both testing data as well as user data.

The PlusLib file changes can be found here.

I will update PlusApp as well.

@lassoan

lassoan commented Feb 1, 2018

Copy link
Copy Markdown
Contributor

If we go and change the names we'll cause inconvenience and may break things, so we should make sure we make things better. Current names are as bad or worse as current ones. TestImages actually stored test images, not user data. We must not store user data in a repository folder. It would show up as local modifications and we might even accidentally add data to the repository. User data should be stored in one of the build output folders, such as c:\D\PlusExp-bin\bin\Release\ (similarly to c:\D\PlusExp-bin\bin\Release\Output).

@adamrankin

Copy link
Copy Markdown
Member Author

Fair enough. I see that the installed folder is data, that will do just fine for me. The build folders can stay TestImages

@adamrankin adamrankin closed this Feb 1, 2018
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.

2 participants