Add pre-cloning-hook for mirror-prepolulation #66
Conversation
polac24
force-pushed
the
bartosz/20240214-pre-clean-clone
branch
2 times, most recently
from
eaeb27b
to
c7fd460
Compare
polac24
force-pushed
the
bartosz/20240214-pre-clean-clone
branch
from
c7fd460
to
02fb622
Compare
lib/git-fastclone.rb
Outdated
| private def trigger_pre_clone_hook(url, mirror, attempt_number) | ||
| return if Dir.exist?(mirror) || !options.include?(:pre_clone_hook) | ||
|
|
||
| popen2e_wrapper(options[:pre_clone_hook], url, mirror, attempt_number.to_s, quiet: !verbose) |
There was a problem hiding this comment.
Do we need to stringify the url and mirror params as well? I see are doing that above on the clone operation. If we know they'll be strings, then this is better, but I wanted to check.
There was a problem hiding this comment.
Sure, I can do that but I checked that in runtime and url and mirror are strings already.
lib/git-fastclone.rb
Outdated
| end | ||
| end | ||
|
|
||
| # Creates or updates the mirror repo then stores an indication | ||
| # that this repo has been updated on this run of fastclone | ||
| def store_updated_repo(url, mirror, repo_name, fail_hard) | ||
| def store_updated_repo(url, mirror, repo_name, fail_hard, attempt_number) | ||
| trigger_pre_clone_hook(url, mirror, attempt_number) |
There was a problem hiding this comment.
We might want to disambiguate which clone operation we're referring to. e.g. trigger_pre_mirror_clone_hook? Should we only call this when we think the clone might actually be called next (e.g. within the unless block)?
There was a problem hiding this comment.
If you think that would be better, let me do that.
There was a problem hiding this comment.
Actually not: now I remember why did I change that logic to a separate method:
lib/git-fastclone.rb:358:5: C: Metrics/CyclomaticComplexity: Cyclomatic complexity for store_updated_repo is too high. [8/7]
I need to revert that to the previous approach
lib/git-fastclone.rb
Outdated
| private def trigger_pre_clone_hook(url, mirror, attempt_number) | ||
| return if Dir.exist?(mirror) || !options.include?(:pre_clone_hook) | ||
|
|
||
| popen2e_wrapper(options[:pre_clone_hook], url, mirror, attempt_number.to_s, quiet: !verbose) |
There was a problem hiding this comment.
Sure, I can do that but I checked that in runtime and url and mirror are strings already.
lib/git-fastclone.rb
Outdated
| end | ||
| end | ||
|
|
||
| # Creates or updates the mirror repo then stores an indication | ||
| # that this repo has been updated on this run of fastclone | ||
| def store_updated_repo(url, mirror, repo_name, fail_hard) | ||
| def store_updated_repo(url, mirror, repo_name, fail_hard, attempt_number) | ||
| trigger_pre_clone_hook(url, mirror, attempt_number) |
There was a problem hiding this comment.
If you think that would be better, let me do that.
Styling changes to invoking pre-clone-hook.
Introducing a new parameter (
--pre-clone-hook) that is called before cloning a repo to the mirror.This feature gives users the opportunity to prepopulate git's mirror from a different source.
Before merging:
git-fastcloneflow. ie. provide a way to indicate if the hook was already triggered in a previous attempt.