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

migrate: switch to git-filter-repo, make temp dir creation more robust, enable changing default branch name #95

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rikkit
Copy link

@rikkit rikkit commented Feb 21, 2022

Hey there, I'm currently using meta to restructure a monorepo I work on, and I've made these changes to meta-project-migrate:

I'm not finished migrating the repo yet - I need to make some more changes to make other meta commands read the defaultBranch option. I've opened this PR now for early feedback since I'm not sure how you would like to review these changes, or if you'd like to accept them all, so let me know and I can split up this PR if needed.

@patrickleet
Copy link
Collaborator

@rikkit

Sorry hadn't seen this before now - usually have an average of 500 notifications lol

I added this migrate command originally - and it worked for my purpose

Is this draft working at least?

@rikkit
Copy link
Author

rikkit commented Apr 7, 2022

Hey, yeah I successfully migrated a couple of projects with the code in here. The two things that were a problem for me with the existing command were

  1. the performance of git filter-repo. Switching to filter-branch means this now takes seconds to migrate
  2. using main instead of master.

@patrickleet patrickleet marked this pull request as ready for review April 8, 2022 21:50
@patrickleet
Copy link
Collaborator

patrickleet commented Apr 8, 2022

the performance of git filter-repo. Switching to filter-branch means this now takes seconds to migrate

Think you said this backwards? 7564b76#diff-b45341acb11c77181833b113debfc339b0893c889afb46121cb1d12c0c0e36d8R5

Anyway, this looks good to me. Performance improvements are always welcome. I'll try it locally.

Thanks!

@rikkit
Copy link
Author

rikkit commented Apr 9, 2022

Think you said this backwards?

Yep, sorry, got confused!

@patrickleet
Copy link
Collaborator

patrickleet commented May 3, 2022

Hi @rikkit

sorry took me a few weeks to get to this...

so, I didn't realize that git filter-repo isn't a standard git command

Using this with filter-repo would require users to install additional tools vs the built in filter-branch

Or am I missing something?

(using git 2.36)

@patrickleet
Copy link
Collaborator

though once installed the migrate command does appear to work

I still see a temp folder that wasn't cleaned up though:

image

@patrickleet
Copy link
Collaborator

oh I think the tmp directory was from the first run where I didn't have filter-repo installed yet, so nevermind on that one

@patrickleet
Copy link
Collaborator

defaultBranch in .meta config works fine though - used main for this test

I guess the only issue is the separate install - hm - I suppose just checking for it and logging an error telling the user to install it would be ok?

@patrickleet
Copy link
Collaborator

@rikkit @mateodelnorte ^

@rikkit
Copy link
Author

rikkit commented May 4, 2022

Hey @patrickleet, thanks for the review and glad it all worked.

I think there are two approaches to take depending on how strongly you feel about it. One checking for it and logging some info on how to install. Two, supporting both versions and falling back to filter-branch if filter-repo is not available.

I feel like approach one is reasonable given that Git have deprecated the command. Plus it's less to maintain in this project.

How should we take this forward?

@patrickleet
Copy link
Collaborator

@rikkit haven't merged it yet, but also added a couple lines yesterday in meta init that will set defaultBranch in the .meta file when you first run it

@patrickleet
Copy link
Collaborator

patrickleet commented May 5, 2022

@rikkit also it's bothered me ever since I made it that I put the folder name before the repo url. I think the other way around would have been better.

Kinda thinking maybe we go with both for now - just change the name of the old command, and add the new one as well using the "migrate" command name (what to put for deprecated one? migrate-classic or something?)

I think the logs should link to instructions of how to install filter-repo for using this version though, regardless, and if you want to use a slower version without installing anything, use migrate-classic. Thoughts?

@stale
Copy link

stale bot commented Aug 13, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Aug 13, 2022
@rikkit
Copy link
Author

rikkit commented Sep 13, 2022

Hey @patrickleet , sorry for the gigantic delay in replying.

Is it possible to specify versions for plugins of meta? Would it work if someone for example just use an older version of meta-project if they want the old behaviour? Maybe by doing npm i -g meta-project@x.x.x. Then the new command can simply print a log message suggesting what to do.

@patrickleet
Copy link
Collaborator

I tried this and it worked, the only real hangup I had was the requirement to install an extra tool and I wasn't really sure about the best thing to do there

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.

None yet

2 participants