-
Notifications
You must be signed in to change notification settings - Fork 16
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
return error instead of panicking upon branch hijacking #168
Conversation
Signed-off-by: Kent <[email protected]>
✅ Deploy Preview for docs-bookkeeper-akuity-io canceled.
|
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #168 +/- ##
==========================================
- Coverage 37.48% 37.26% -0.23%
==========================================
Files 19 19
Lines 1139 1146 +7
==========================================
Hits 427 427
- Misses 695 702 +7
Partials 17 17
☔ View full report in Codecov by Sentry. |
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.
LGTM
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.
LGTM!
Fixes #166
This will prevent the nil pointer dereference that currently occurs when Bookkeeper is asked to write to an existing branch that isn't already Bookkeeper-managed.
It will return an error instead, which seems like the most sensible thing to do, because this was probably a mistake on the user's part.
If you do want Bookkeeper to take over an existing branch, you can either delete the target branch or manually
touch .bookkeeper/metadata.yaml
on the target branch.cc @jessesuen @christianh814 this tripped up both of you (and me too) at some point in recent weeks.