-
-
Notifications
You must be signed in to change notification settings - Fork 176
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
Prevent no branches #1074
base: master
Are you sure you want to change the base?
Prevent no branches #1074
Conversation
This pull request fixes 1 alert when merging f2bb649 into 5d3c500 - view on LGTM.com fixed alerts:
|
Sorry I think I broke the CI with é in my last name 😅 |
Yeah. Perhaps my script could be setup to decode utf8. Try changing |
src/branches.cpp
Outdated
@@ -269,6 +272,10 @@ namespace l | |||
int | |||
erase_begin(Branches::Impl *branches_) | |||
{ | |||
if (branches_->size() <= 1) { |
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.
If you don't mind updating the code to use a more GNU style for braces and such. And I'm fine with something like
if(branches_->size() <= 1)
return -ENOTSUP;
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.
OK, updated the code to better match the style
f2bb649
to
ece486d
Compare
This pull request fixes 1 alert when merging ad6c93f into 5d3c500 - view on LGTM.com fixed alerts:
|
This pull request fixes 1 alert when merging 5d61a5b into 5d3c500 - view on LGTM.com fixed alerts:
|
5d61a5b
to
80e0752
Compare
This pull request fixes 1 alert when merging 80e0752 into 5d3c500 - view on LGTM.com fixed alerts:
|
f90ab5f
to
694b819
Compare
This pull request fixes 1 alert when merging 694b819 into 5d3c500 - view on LGTM.com fixed alerts:
|
694b819
to
f283cba
Compare
This pull request fixes 1 alert when merging f283cba into 5d3c500 - view on LGTM.com fixed alerts:
|
Ugh. Sorry. Python is a major pita regarding char encoding. I can try to fix it and then you can submit the PR for the original issue. Or if you want to see it through feel free. |
Can't you just set the ENV to UTF-8 encoding? Either through PYTHONIOENCODING="UTF-8" or the LANG/LANGUAGE/LC_ALL? |
as discussed in #1073, this seems to fix it.
returns "not supported" ENOTSUP when a change would result in no branches.
incidentally, the code for branches
from_string
was already raising/erroring out in case of setting completely empty branches, that's fixed. I've added unit test cases to test all that.On my system, the
-fsanitize=undefined
option in the compiler added some problems that seem to be fixed by also adding the-fsanitize=undefined
flag on linking, not sure why, that's why the change is in there,