-
Notifications
You must be signed in to change notification settings - Fork 74
Fixups and improvements to git info retrieval #991
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR refactors and improves the git info retrieval logic and tests, including better handling for repositories without an upstream and updates to the returned tuple in get_git_info.
- Refactored git info functions into get_git_root, get_git_remote_info, get_git_repo_info, and updated get_git_info to return an additional absolute path value.
- Updated related tests to match the new interface and behavior.
- Adjusted tuple unpacking in base_storage to accommodate the new return value.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
mlos_bench/mlos_bench/util.py | Refactored git info retrieval functions and updated get_git_info return structure. |
mlos_bench/mlos_bench/tests/util_git_test.py | Modified and added tests to validate the updated git info retrieval behavior. |
mlos_bench/mlos_bench/storage/base_storage.py | Updated tuple unpacking from get_git_info to capture the additional absolute path element. |
Comments suppressed due to low confidence (2)
mlos_bench/mlos_bench/storage/base_storage.py:197
- [nitpick] The variable '_future_pr' is ambiguous; consider renaming it to '_abs_path' or a similar descriptive name that indicates it holds the absolute path returned by get_git_info.
(self._git_repo, self._git_commit, self._root_env_config, _future_pr) = get_git_info(
mlos_bench/mlos_bench/util.py:401
- The return tuple description in the docstring could be clarified by explicitly separating the four elements (e.g., by inserting commas) to clearly indicate that abs_path is an additional value.
(git_repo, git_commit, rel_path, abs_path) : tuple[str, str, str, str]
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Co-authored-by: Sergiy Matusevych <[email protected]>
Pull Request
Title
Fixups and improvements to git info retrieval
Description
Various fixups and improvements to git info retrieval:
Test and handle cases where the local git repo has no upstream or the upstream is not "origin".
Prepare to also return the absolute path of the config file as well.
Type of Change
Testing
Additional Notes (optional)
To be merged before #985