Added shouldIgnoreFileURL:error: convenience wrapper #545
Conversation
…Status for compatibility with Swift; closes libgit2#541
|
For my feeling this is a lot of bool comparison to go right the first time. Could you add a test for the new method? |
| @@ -125,4 +125,10 @@ - (BOOL)shouldFileBeIgnored:(NSURL *)fileURL success:(BOOL *)success error:(NSEr | |||
| return (ignoreState == 1 ? YES : NO); | |||
| } | |||
|
|
|||
| - (GTFileIgnoreState)shouldIgnoreFileURL:(NSURL *)fileURL error:(NSError **)error { | |||
| BOOL success = false; | |||
There was a problem hiding this comment.
Conventionally this should be NO instead of false.
There was a problem hiding this comment.
Ha; guess it shows that I've been coding nothing but Swift for the past while...
| - (GTFileIgnoreState)shouldIgnoreFileURL:(NSURL *)fileURL error:(NSError **)error { | ||
| BOOL success = false; | ||
| BOOL ignore = [self shouldFileBeIgnored:fileURL success:&success error:error]; | ||
| return ignore && success ? GTFileIgnoreStateShouldIgnore : (!ignore && success ? GTFileIgnoreStateShouldNotIgnore : GTFileIgnoreStateIgnoreCheckFailed); |
There was a problem hiding this comment.
This is a bit complex to squeeze into one ternary. How about splitting them up?
|
Thanks @onecrayon! This is close, just a couple comments.
Yup, a test or two would be great 👍 |
|
I am not familiar with the testing framework being used here, and there does not appear to be any test associated with the method this is wrapping, so mimicking that is out (e.g. -statusForFile:success:error). I've been testing this locally, and the logic is fine (you'll note my previous pull request fixing the broken logic in the method I'm wrapping...), but if you still want a test added to the suite you'll need to point me in the direction of a good example that I can customize for this particular method. |
|
True, the existing implementation is not being tested directly. There are some status specs here you could just copy paste one if them and check if your method returns the right enum value. |
…TRepository.shouldIgnoreFileURL:error:
|
Added a couple of tests: one for the original method and one for the wrapper; both are passing for me. Although FYI the testing suite as a whole is crashing and burning on a memory error for an unrelated test later on: |
|
Looks good, thanks! |
Added shouldIgnoreFileURL:error: convenience wrapper
Added to GTRepository+Status for compatibility with Swift; closes issue #541.