search

CarnegieLearningWeb / educational-experiment-service

Service to manage educational experiments, part of the UpGrade Platform
https://www.upgrade-platform.org/
BSD 3-Clause "New" or "Revised" License
5 stars 7 forks source link

Consolidate repos #97

Open amurphy-cl opened 3 years ago

amurphy-cl commented 3 years ago

Can we consolidate repos under one top level so people can have one link under which all UpGrade code is hosted?

jerith666 commented 3 years ago

I'm happy to create a new CarnegieLearningWeb/educational-experiments (or CarnegieLearningWeb/UpGrade) repo. But @mfugate1 and/or the PlayPower devs will need to tell us if there are technical reasons that separate repos for the front- and back-end are needed. I honestly can't recall why we set it up this way initially.

A less technically disruptive option might be to simple put something like this in the README.md files at the root of each separate repo:

Welcome to the UpGrade project!  The project's code is divided into the following repositories:

* educational-experiment-client: <one-sentence description>
* educational-experiment-service: <one-sentence-description>  <-- You Are Here
* educational-experiment-types: <one-sentence-description>
* ...

<more detailed description of this repo>

cc @SritterCL

mfugate1 commented 3 years ago

I wasn't involved in the initial planning so I'm not sure what the reason for separating them was, but as far as I know, they could be combined into a single repo. I would just need to make a few changes to the github actions that build and deploy everything to support that, but I don't think it would take me very much time to do that.

nirmalpatel commented 3 years ago

I'm wondering how this will affect our version bumping setup...

On Thu, Jan 14, 2021 at 10:26 PM mfugate1 notifications@github.com wrote:

I wasn't involved in the initial planning so I'm not sure what the reason for separating them was, but as far as I know, they could be combined into a single repo. I would just need to make a few changes to the github actions that build and deploy everything to support that, but I don't think it would take me very much time to do that.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/CarnegieLearningWeb/educational-experiment-service/issues/97#issuecomment-760324992, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGKXQBIXJZS2TPOXDBJX3DSZ4O3VANCNFSM4WBLDKSA .

mfugate1 commented 3 years ago

That's part of the change I'd make in the github actions. I can specify which package.json files to bump based on which files were changed

nirmalpatel commented 3 years ago

That sounds great. I don't recall the real reason behind separate repositories... @JD2455 your thoughts?

amurphy-cl commented 3 years ago

Just adding a vote for the main repo to be renamed CarnegieLearningWeb/UpGrade.

nirmalpatel commented 3 years ago

Chatted with JD and it sounds like there are no issues with consolidation on our side.

jerith666 commented 3 years ago

Cool. It'd be great to preserve the git history in the new combined repo; do you and JD have any experience doing that?

JD2455 commented 3 years ago

@jerith666 We don't really have any experience of doing that.

jerith666 commented 3 years ago

I'll share our internal write-up on how to do it.

jerith666 commented 3 years ago

I shared our write-up back in January. I've now created the https://github.com/CarnegieLearningWeb/UpGrade repo and granted access to everyone who's got access here.

I don't really know how to weave together multiple typescript repos, so I'm assigning this back to @nirmalpatel to delegate to someone at PlayPower.

nirmalpatel commented 3 years ago

@jerith666 Can you re-share your writeup? I cannot find it in my January emails...

@amurphy-cl Do you have a timeline for this? We are tackling load testing with Locust this week...

jerith666 commented 3 years ago

@nirmalpatel it's in slack #playpower channel on Jan. 19th; I'll ping you there so it lights up for you.

amurphy-cl commented 3 years ago

@nirmalpatel If you can do it next week after focusing on Locust testing this week that would be great. My impression was that this sort of thing would not take very long but if it does interfere with other things too much, please let me know.

amurphy-cl commented 3 years ago

Any blockers on this issue? Let's try to get it completed.

nirmalpatel commented 3 years ago

@amurphy-cl team was working on Locust and 500 Internal Error issues and they are now almost resolved. This is next in line.

jerith666 commented 3 years ago

I've reviewed the structure created by https://github.com/CarnegieLearningWeb/UpGrade/commit/7e582846fa960534f1ff2c1ace79d21d718efe09 and its parents (more precisely, git log 7e582846 ^acdfdd36 ^e6d5272f ^d7cf5dcb), and it looks right to me with one big exception: the code from https://github.com/CarnegieLearningWeb/educational-experiment-types does not seem to be present anywhere. I'm not sure how other parts of the code are working without this ... ?

nirmalpatel commented 3 years ago

@jerith666 I checked on creating pull requests across the repositories but I don't think there is a way to do that. You can only compare across forks of a repository...

image

So I'm not sure how to merge the current pull requests that we have in the backend repository. Any ideas on how to do this? If there is no way, maybe we can go code freeze route...

jerith666 commented 3 years ago

Oh, sorry, I should've been more clear in my comments in last week's status meeting: I meant that it should work fine to merge branches across repos on the command line. I'm not at all surprised that GitHub doesn't support this very esoteric use case in their GUI.

But if you still have the multiple-remotes repo set up from the initial merge, you should just be able to fetch + merge in there with little trouble.

nirmalpatel commented 3 years ago

@jerith666 @mfugate1 So I gave this a try where I tried to bring the latest code commit in our backend's master branch into the new repo and I'm having an issue at an intermediate step. I followed this post: https://stackoverflow.com/questions/39751106/how-to-merge-git-repositories-in-subfolders-without-losing-branches-and-tags

I can get all of the branches of the backend repo into the mono repo.

git fetch --no-tags backend refs/heads/*:refs/heads/backend/* refs/tags/*:refs/tags/backend/*

image

Then I can do a merge but I get some warnings:

image

And when I try to read the tree into the index, I get an error which I'm not sure how to resolve:

image

Do you have any idea what I can do here?

jerith666 commented 3 years ago

You don't need to use low-level commands like read-tree now that you've done the initial work of creating the combined history. With the history connected as it is, git is able to handle the merge as a normal merge:

$ git status
On branch upgrade-main
Your branch is up to date with 'upgrade/main'.

nothing to commit, working tree clean

$ git rev-parse HEAD
ddda0d029e72ee43452a282dddc4ce1be361536f

$ git merge ees/master 
CONFLICT (modify/delete): packages/Upgrade/package-lock.json deleted in HEAD and modified in ees/master. Version ees/master of packages/Upgrade/package-lock.json left in tree.
CONFLICT (file location): packages/Upgrade/test/integration/EndingCriteria/index.ts added in ees/master inside a directory that was renamed in HEAD, suggesting it should perhaps be moved to backend/packages/Upgrade/test/integration/EndingCriteria/index.ts.
CONFLICT (file location): packages/Upgrade/test/integration/EndingCriteria/ParticipantsOnly.ts added in ees/master inside a directory that was renamed in HEAD, suggesting it should perhaps be moved to backend/packages/Upgrade/test/integration/EndingCriteria/ParticipantsOnly.ts.
CONFLICT (file location): packages/Upgrade/test/integration/EndingCriteria/GroupAndParticipants.ts added in ees/master inside a directory that was renamed in HEAD, suggesting it should perhaps be moved to backend/packages/Upgrade/test/integration/EndingCriteria/GroupAndParticipants.ts.
Auto-merging backend/packages/Upgrade/package.json
CONFLICT (content): Merge conflict in backend/packages/Upgrade/package.json
Recorded preimage for 'backend/packages/Upgrade/package.json'
Automatic merge failed; fix conflicts and then commit the result.

$ git status
On branch upgrade-main
Your branch is up to date with 'upgrade/main'.

You have unmerged paths.
  (fix conflicts and run "git commit")
  (use "git merge --abort" to abort the merge)

You are in a sparse checkout with 100% of tracked files present.

Changes to be committed:
        modified:   backend/packages/Upgrade/nodemon.json
        modified:   backend/packages/Upgrade/src/api/models/ExperimentPartition.ts
        modified:   backend/packages/Upgrade/src/api/models/MonitoredExperimentPoint.ts
        modified:   backend/packages/Upgrade/src/api/services/ExperimentAssignmentService.ts
        modified:   backend/packages/Upgrade/src/api/services/ExperimentService.ts
        modified:   backend/packages/Upgrade/test/integration/index.test.ts
        modified:   backend/packages/Upgrade/test/integration/mockData/experiment/index.ts

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
        both modified:   backend/packages/Upgrade/package.json
        added by them:   backend/packages/Upgrade/test/integration/EndingCriteria/GroupAndParticipants.ts
        added by them:   backend/packages/Upgrade/test/integration/EndingCriteria/ParticipantsOnly.ts
        added by them:   backend/packages/Upgrade/test/integration/EndingCriteria/index.ts
        deleted by us:   packages/Upgrade/package-lock.json

Then you can proceed to resolve the conflicts as you usually would. (And don't forget to include information on what caused the conflict and how it was resolved in the merge commit message! :-) )

Note that you'll want a relatively new version of git to make sure it marks those "suggesting it should perhaps be moved to" files as conflicting.

My remotes are named slightly differently than yours, but otherwise my setup is the same:

$ git remote -vv | egrep UpGrade\|educational-experiment-service.git
ees     git@github.com:CarnegieLearningWeb/educational-experiment-service.git (fetch)
ees     git@github.com:CarnegieLearningWeb/educational-experiment-service.git (push)
upgrade git@github.com:CarnegieLearningWeb/UpGrade (fetch)
upgrade git@github.com:CarnegieLearningWeb/UpGrade (push)
msandbothe commented 3 years ago

merged client code into consolidated repo. Checked libs as well, but no missing changes.

nirmalpatel commented 3 years ago

Thanks @msandbothe ! We will start working off of the new repo now.

amurphy-cl commented 3 years ago

Checking if there are any last bits of code that need to be moved to the UpGrade repo before I close this issue. Are we good?

nirmalpatel commented 3 years ago

@amurphy-cl I think we are good, we are not going to work on the old repos anymore. We can leave them there for now...

jerith666 commented 3 years ago

It might be worth making a commit in each of the old repos that removes all code and updates the README to say that the code has moved.

mfugate1 commented 3 years ago

Or I can just archive the old repos instead.

jerith666 commented 3 years ago

I wasn't sure how worried we were about people elsewhere on the web having bookmarked the old repos etc. If we're not, then sure, archiving sounds better.