Skip to content

Conversation

bpkroth
Copy link
Contributor

@bpkroth bpkroth commented Jun 24, 2025

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

  • 🛠️ Bug fix
  • 🔄 Refactor
  • 🧪 Tests

Testing

  • CI
  • New unit tests

Additional Notes (optional)

To be merged before #985


@Copilot Copilot AI review requested due to automatic review settings June 24, 2025 13:53
@bpkroth bpkroth requested a review from a team as a code owner June 24, 2025 13:53
@bpkroth bpkroth added bug Something isn't working ready for review Ready for review refactor labels Jun 24, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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]

@bpkroth bpkroth changed the title Fixup/git info Fixups and improvements to git info retrieval Jun 24, 2025
@bpkroth bpkroth enabled auto-merge (squash) June 24, 2025 18:20
Co-authored-by: Sergiy Matusevych <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready for review Ready for review refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants