-
Notifications
You must be signed in to change notification settings - Fork 95
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
Incorporate Recent Commits #192
Conversation
Fix git method & add more tests
Add Support for Local and Remote Repositories
Still a draft. Don't merge. |
About the CacheHere use relative paths in Original Method: def get(self, url: str, commit: str) -> Optional[Path]:
_, repo_name = _split_git_url(url)
dirname = _format_dirname(url, commit)
def store(self, src: Path) -> Path:
url, commit = get_repo_info(src)
dirpath = self.cache_dir / _format_dirname(url, commit)
_, repo_name = _split_git_url(url)
# Cache the traced repo
src_dir = tmp_dir / repo.name
path = cache.store(src_dir) Modified Version: def get(self, rel_cache_dir: Path) -> Optional[Path]:
dirname = rel_cache_dir.parent
dirpath = self.cache_dir / dirname
cache_path = self.cache_dir / rel_cache_dir
def store(self, src: Path, rel_cache_dir: Path) -> Path:
"""Store a repo at path ``src``. Return its path in the cache.
Args:
src (Path): Path to the repo.
rel_cache_dir (Path): The relative path of the stored repo in the cache.
"""
dirpath = self.cache_dir / rel_cache_dir.parent
cache_path = self.cache_dir / rel_cache_dir
# Cache the traced repo
src_dir = tmp_dir / repo.name
rel_cache_dir = repo.format_dirname / repo.name
path = cache.store(src_dir, rel_cache_dir) The function Moreover, $HOME/.cache/lean_dojo
├── aesop-79fb157c6a5061190d169535f8e5cb007914a82e
│ └── aesop
├── batteries-e7897807913fafdab31b01b9f627550bcc96cff2
│ └── batteries
...
├── repos
│ ├── gitpython-aesop-79fb157c6a5061190d169535f8e5cb007914a82e
│ ├── gitpython-batteries-e7897807913fafdab31b01b9f627550bcc96cff2 For git repo, it uses a fixed prefix ( btw, the |
A note about the cache that might be helpful for the review process. Feel free to @ me if you get any questions😃 |
|
||
|
||
def cleanse_string(s: Union[str, Path]) -> str: | ||
"""Replace : and / with _ in a string.""" | ||
return str(s).replace("/", "_").replace(":", "_") | ||
|
||
|
||
@cache | ||
def _to_commit_hash(repo: Repository, label: str) -> str: | ||
def _to_commit_hash(repo: Union[Repository, Repo], label: str) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RexWzh The new version of _to_commit_hash
no longer caches the results for GitHub repos?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, I omitted that line when updating the function. You can reintroduce it.
I guess this is why the latest tests take much time.
No description provided.