Closed asmagill closed 3 months ago
Assuming CI "passes" with only its regular amount of failing tests, this lgtm.
Attention: Patch coverage is 94.20290%
with 4 lines
in your changes missing coverage. Please review.
Project coverage is 27.95%. Comparing base (
ec9a8ea
) to head (e86ba5c
).
The regular amount of tests failed, so I'm merging this.
Was just about to add comments in the internal pushNSNumber:
method for when we migrate to Swift.
We currently do a ptr comparison against kCFBooleanTrue
and kCFBooleanFalse
to determine if an NSNumber
is boolean (@encode for BOOL
is the same as for unsigned char
, so we can't use that).
Per https://stackoverflow.com/a/30223989, it's probably safer (and Swift friendly) to do something like:
func isBoolNumber(num:NSNumber) -> Bool
{
let boolID = CFBooleanGetTypeID() // the type ID of CFBoolean
let numID = CFGetTypeID(num) // the type ID of num
return numID == boolID
}
They give an Obj-C version as well, but I don't want to change what's currently working atm.
If we have a place where we're sticking Swift related notes, let me know and I'll post this there.
We don't have such a thing yet, but we probably should. I wonder if a Project might make sense?
Some proposed changes to LuaSkin:
I know we're planning a move to Swift soon, but in the meantime, this pull has the following:
1) the
pushNSObject:
methods will now check for a more specific match (i.e.isMemberOfClass:
) and then fall back toisKindOfClass:
.2) the
log*:
methods will now accept varagrs in theNSString stringWithFormat:
style.[LuaSkin logError:@"%s - callback error", USERDATA_TAG]
, as opposed to the current[LuaSkin logError:[NSString stringWithFormat:@"%s - callback error", USERDATA_TAG]]