Open hollandjg opened 1 month ago
I appreciate the intention behind the proposed changes! However, I have some concerns about moving the core package files to a subfolder.
If we intend to publish this package to a registry (such as the Julia Registry for greater visibility), it’s essential that the core code files, README, Project.toml, supporting documentation, and License files remain at the root of the repository. This is a standard convention in the Julia ecosystem to ensure proper visibility and accessibility for users and registries alike.
Even if we don’t plan to add this project to a registry, pointing to a subfolder can complicate the installation process. Users would need to navigate to the repository page, click on the folder containing the code, and then copy the link to that folder. Alternatively, they would have to clone the entire repository and specify a subfolder for installation, which adds unnecessary complexity.
I believe these drawbacks outweigh the benefit of reducing the number of items in the root directory. What are your thoughts?
I appreciate the intention behind the proposed changes! However, I have some concerns about moving the core package files to a subfolder.
Thanks for the feedback!
If we intend to publish this package to a registry (such as the Julia Registry for greater visibility), it’s essential that the core code files, README, Project.toml, supporting documentation, and License files remain at the root of the repository. This is a standard convention in the Julia ecosystem to ensure proper visibility and accessibility for users and registries alike.
The repo IFTPipeline.jl
as it exists today isn't a clean Julia package. The Julia part is mixed up with a whole lot of other stuff, including the cylc configuration, the fetchdata.sh
script, and so on, which aren't relevant for understanding the Julia part, and which we perhaps wouldn't want to be included in a registry. I want to make it easy to understand what is where.
It was really difficult for me to understand what was related and what could be ignored, and making this move helped me start to draw the boundaries between the different components. I think I understand now which Project.toml file is to be used for which thing.
Partitioning the code more cleanly is a necessary shim for the refactoring I'm doing. Whether once we've refactored and tested the code we move it back so it's more Julian is a different question. I'm happy to put that in as an issue.
Even if we don’t plan to add this project to a registry, pointing to a subfolder can complicate the installation process. Users would need to navigate to the repository page, click on the folder containing the code, and then copy the link to that folder. Alternatively, they would have to clone the entire repository and specify a subfolder for installation, which adds unnecessary complexity.
I don't think users beyond the research group are going to be using this version, because it's so hard to get the Python dependencies working, and it's not been really tested yet. I think we need to provide Docker images which can deal with the complicated Conda dependencies. I'd be happy to refactor it again later to help those users.
It looks like Pkg supports Pkg.add(PackageSpec(path="MainRepo", subdir="SubDir.jl"))
now: https://github.com/JuliaLang/Pkg.jl/pull/1422
... but I'm not sure if this feature has seen widespread adoption.
Some more things to think about, and let's assume that we keep the original structure IFTPipeline.jl/src
:
IFTPipeline.jl/PythonSetup.jl
. Ideally we'd get rid of the python dependency completely and get rid of that problem, but it's a problem we have it right now. Having a separate subdirectory is a good way to solve that for the minute, whilst we're refactoring.I believe these drawbacks outweigh the benefit of reducing the number of items in the root directory. What are your thoughts?
Here are the options I think we need to decide between (in my descending order of preference):
... to help clean up the repository. The goal is to have neatly separated directories with:
The CLI file is intentionally ignored here, and will be moved and refactored in #174
Part of:
149