Skip to content

Conversation

james-bruten-mo
Copy link
Contributor

Description

Add a new class to bdiff as somewhere to get other info about a branch. For now include a function testing whether the branch provided is "main-like". This is useful for some UM tests, where we want to decide whether to run over a branch diff or the full trunk.

Checklist

  • I have performed a self-review of my own changes

@james-bruten-mo james-bruten-mo requested a review from a team as a code owner August 29, 2025 15:24
@james-bruten-mo james-bruten-mo requested review from Pierre-siddall, mo-nikosbaltas and t00sa and removed request for a team, Pierre-siddall and mo-nikosbaltas August 29, 2025 15:24
@james-bruten-mo james-bruten-mo mentioned this pull request Sep 1, 2025
1 task
Copy link
Contributor

@t00sa t00sa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See suggestion about testing.

Comment on lines +71 to +76
for line in self.run_git(["branch"]):
if "HEAD detached" in line:
result = self.detached_head_reference
break
else:
raise GitBDiffError("unable to get branch name")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This chunk of code isn't currently being tested. I think it should be easy to check out a detached head in the test repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Have also added a couple of init.py files and a relative import, so now the unit tests can be run by just pytest from the bdiff directory

Copy link
Contributor

@Pierre-siddall Pierre-siddall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code for this looks great, I have one small suggestion regarding documenting the use of a walrus operator as this is something I had to look up otherwise I'm happy to hit approve on this.

Copy link
Contributor

@t00sa t00sa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Copy link
Contributor

@Pierre-siddall Pierre-siddall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This walrus is now very happy. Approved.

@Pierre-siddall Pierre-siddall merged commit efc37b4 into MetOffice:main Sep 3, 2025
5 checks passed
@james-bruten-mo james-bruten-mo deleted the bdiff_info branch September 3, 2025 07:47
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.

3 participants