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

WIP: Extract GEOSldas_GridComp and LDAS_App into separate repos #712

Closed
wants to merge 2 commits into from

Conversation

mathomp4
Copy link
Member

@mathomp4 mathomp4 commented Feb 16, 2024

This is a work-in-progress PR for extracting out the GEOSldas_GridComp and LDAS_App directories into their own components (for work with @saraqzhang and @rtodling in integrating LDAS into the ADAS).

Using RepoExtractor, the two repos were extracted with:

./extract_repo.bash -r "GEOSldas" -d "LDAS_App" --newrepo "LDAS_App" --develop --create-repo --push
./extract_repo.bash -r "GEOSldas" -d "GEOSldas_GridComp" --newrepo "GEOSldas_GridComp" --develop --create-repo --push

In my test it built. Which is nice.


NOTE 1: This should not be depended on. Most likely the two new repos will need to be deleted and remade once this is done "for real"

NOTE 2: In this PR, I kept GEOSgcm_GridComp underneath GEOSldas_GridComp. It could be moved parallel as well. CMake doesn't care, but in this way the directory tree before and after will look nigh the same

@mathomp4 mathomp4 self-assigned this Feb 16, 2024
@mathomp4
Copy link
Member Author

I suppose up next would be to have @biljanaorescanin test to see if this does more than build! :)

@gmao-rreichle
Copy link
Contributor

@mathomp4, thanks for putting this PR and the Github Project together, very helpful. I'm starting to understand what this all means. Here are a couple of initial thoughts:

  1. For consistency with the nomenclature of GEOSgcm, I suggest calling the "App" repo "GEOSldas_App" (not "LDAS_App"). This makes it more obvious to developers that GEOSldas, GEOSldas_App, and GEOSldas_GridComp all go together.

  2. I'm wondering if there's any chance we can limit the split to just two repos, GEOSldas (fixture) and GEOSldas_App_and_GC. I recognize that this is not following the current template across other GEOS repos, and it won't neatly fit into the directory tree of GEOSadas. But the GEOSadas and GEOSldas fixtures need both the GEOSldas_App and the GEOSldas_GridComp elements, so importing them from separate repos does not seem strictly necessary. Put differently, the motivation for having three repos in total seems to be coming only from having split other components (e.g., the GCM) into separate "App" and "GC" repos. I might be missing something, of course, but I thought I should at least ask before committing myself to start juggling lots of extra LDAS PRs.

cc: @tclune @weiyuan-jiang @rtodling

@tclune
Copy link
Contributor

tclune commented Feb 19, 2024

@gmao-rreichle You are the closest to the issue. I have no strong opinions. I will point out that it is easier to split again later on than to undo a split that you come to regret.

@tclune tclune closed this Feb 19, 2024
@tclune tclune reopened this Feb 19, 2024
@mathomp4
Copy link
Member Author

@gmao-rreichle I mean, there is no reason that LDAS_App/ is in Applications/ other than that's how it was in CVS and when we went to Git, we tried to emulate the directory structure.

We could put LDAS_App/ inside of the GEOSldas_GridComp repo, I think. CMake doesn't care. But I'd have to do some testing to figure out if the tool I use to extract the history of it cares.

My guess is the best way to do this is I'd first make a PR to GEOSldas where all we do is a git mv of Applications/LDAS_App into Components/GEOSldas_GridComp/GEOSldas_App (and I guess delete the Applications/ folder as well. I'll make a quick draft PR of that just to see what happens with the history here on GitHub.

@mathomp4
Copy link
Member Author

Update: Okay. I can also do it in my extraction process too. The one thing is it looks like that way the history is not retained. This might not be a bad thing, we can just tell people this history for GEOSldas_App is in the GEOSldas repo.

But I'll keep trying things...

@mathomp4
Copy link
Member Author

This PR will be superseded by a new one. Closing.

@mathomp4 mathomp4 closed this Mar 25, 2024
@mathomp4 mathomp4 deleted the feature/mathomp4/extract-ldas branch March 25, 2024 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants