-
Notifications
You must be signed in to change notification settings - Fork 22
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
Add test for LibGAP.jl, fix bug in Base.string method #148
Conversation
Codecov Report
@@ Coverage Diff @@
## master #148 +/- ##
==========================================
- Coverage 56.44% 55.29% -1.16%
==========================================
Files 44 42 -2
Lines 2599 2465 -134
==========================================
- Hits 1467 1363 -104
+ Misses 1132 1102 -30
|
a1f7a48
to
c644f3d
Compare
@@ -13,7 +13,7 @@ function Base.show( io::IO, obj::Union{MPtr,GAP.GapFFE} ) | |||
end | |||
|
|||
function Base.string( obj::Union{MPtr,GAP.GapFFE} ) | |||
str = GAP.String( obj ) | |||
str = GAP.Globals.String( obj ) | |||
return CSTR_STRING( str ) |
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.
Nitpick: Shouldn't we use from_gap
here?
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.
I think it's fine either way, but I'd prefer to merge it as-is, with minimal changes. But feel free to edit this PR, or change it in another PR.
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.
It can be changed later, I guess.
No description provided.