phetsims / vegas

Reusable game components for PhET simulations.
MIT License
1 stars 4 forks source link

Move StatusBar to scenery-phet #101

Closed samreid closed 2 years ago

samreid commented 2 years ago

For https://github.com/phetsims/center-and-spread/issues/6 @chrisklus and I discussed with @pixelzoom about moving StatusBar to scenery-phet and agreed it is a good plan.

I'll keep the history.

samreid commented 2 years ago

I observed that the command ../perennial/bin/copy-history-to-different-repo.sh js/StatusBar.ts ../scenery-phet/ failed to get the history prior to the file rename: Here is the history in vegas:

image

Here is the history in scenery-phet after the copy:

image

@zepumph or @pixelzoom any idea how to get the history across that file rename?

samreid commented 2 years ago

I tried making up my own multi-script using the patches idea, like so:

git log --pretty=email --patch-with-stat --reverse --full-index --binary -- js/StatusBar.ts > ../patches/patch1.txt
git log --pretty=email --patch-with-stat --reverse --full-index --binary -- js/StatusBar.js > ../patches/patch2.txt

cd ../scenery-phet
git am --whitespace=fix --reject < ../patches/patch1.txt
git am --whitespace=fix --reject < ../patches/patch2.txt

However, this failed and I also tried applying patch2 then patch1 and it failed for a different reason. In one case, I ended up with a TS file and a JS file, where the JS file had some history, but still ended with define() implementations.

samreid commented 2 years ago

I have a proposed plan that I think will work well for this issue and future cases where we want to move history from one repo to another more robustly.

  1. Install the command git filter-repo. It is recommended by the git documentation as an improvement over git filter-branch. https://git-scm.com/docs/git-filter-branch#_warning. I used git --exec-path to see the path for auxiliary git commands. On my platform it was /Library/Developer/CommandLineTools/usr/libexec/git-core

  2. Clone another copy of the repo we are copying from. In this case, it is vegas.

    git clone https://github.com/phetsims/vegas.git vegas-backup
  3. Filter the repo so it only contains the files of interest:

cd vegas-backup
git filter-repo --path js/StatusBar.js --path js/StatusBar.ts
  1. Remove unwanted branches from the source repo. For example, remove all branches except master.
git branch | grep -v "master" | xargs git branch -D 
  1. Inspect the filtered source repo to make sure it is what we want to merge to the target repo.
  2. Merge the source repo into the target repo. This can be done fully locally, using a local directory for git remote add. Follow directions for doing a local repo merge, like: https://stackoverflow.com/questions/56051560/how-to-merge-two-git-repositories-without-loosing-the-history-for-either-reposit which is basically the following. But please use a sensible name for the remote nickname, since it will show in the commit history.
cd ../scenery-phet
git remote add repo-b ../vegas-backup
git fetch repo-b
git merge repo-b/master --allow-unrelated
git remote remove repo-b
  1. Inspect the target repo and make sure it now has the desired commits/files/history. Update the namespace and registry statement, if we are still using that. See https://github.com/phetsims/phet-core/issues/100. Type check and test the new code. Preview what files are about to be pushed with git diff --stat --cached origin/master . Then git push.
  2. Remove the filtered clone, so you don't accidentally push it later on: rm -rf vegas-backup.
  3. Remove the old copy of the source file, commit and push.
samreid commented 2 years ago

Following the above instructions, I moved StatusBar and its history and renames from vegas to scenery-phet. I cloned a fresh version of scenery-phet and confirmed the history looked as desired. I updated all references to point to the version in scenery-phet. I removed the old copy from vegas. I tested Center and Variability, and it looked good. I tested Build an Atom, which creates one status bar. I checked the CT "quick bar" a few minutes later, and it was all green. Type checking and linting are passing on my side. I'll check full CT columns in the next few business hours.

UPDATE: I wrote on slack:

I pushed a different kind of commits when moving history from one repo to another. Local testing and quick CT looks good. Context is StatusBar and https://github.com/phetsims/vegas/issues/101#issuecomment-1093659193 in case you see trouble. I’ll check CT columns in the morning.

samreid commented 2 years ago

@pixelzoom can you please review this change? In particular, can you please check that the history of StatusBar in scenery-phet is intact? If you additionally have time to review the process in https://github.com/phetsims/vegas/issues/101#issuecomment-1093659193, that would be great. If not, please assign to @zepumph to check on that part.

pixelzoom commented 2 years ago

Commit history for StatusBar.ts looks like it's intact, and I see commit back to 4/13/18. FYI, it began its life on 4/13/18 when it was factored out as a base class for https://github.com/phetsims/vegas/issues/66.

The process for moving git history described in https://github.com/phetsims/vegas/issues/101#issuecomment-1093659193 looks reasonable, but I'm not certain. So it would be good to have @zepumph take a look too. I thought there was a process for this, but maybe I'm thinking of the process for restoring lost git history. In any case, it would be useful to have both of those processes describes in a document --- maybe a new doc in phetinfo for "git tips".

samreid commented 2 years ago

I thought there was a process for this,

The established process in perennial/bin/copy-history-to-different-repo.sh did not support the file rename, even when I tried to adapt it to accommodate the rename.

pixelzoom commented 2 years ago

@samreid How about moving the "copy history to different repo" issue to a new GitHub issue? It's not really related to StatusBar, and having it in its own issue will make it easier to find in the future.

samreid commented 2 years ago

Thanks, I opened https://github.com/phetsims/perennial/issues/269. Since the StatusBar move has been reviewed, this issue can be closed.