Conversation
* Enable -Wmissing-declarations and -Werror to ensure all global functions are defined with a proper declaration. JNI function not found problem can only be seen at run time, thus we need to do so. * Add static keyword for local functions. * Fix wrong JNI function declaration. * Remove useless functions.
| return 0; | ||
| } | ||
|
|
||
| JNIEXPORT jboolean JNICALL Java_io_realm_internal_Group_nativeEquals( |
There was a problem hiding this comment.
Is this useless? Seems more like Group.equals() should actually use it instead?
There was a problem hiding this comment.
Do we ever need to call Group.equals()?
There was a problem hiding this comment.
We don't right now at least, and Group might go away with the object store, so yeah, ¯_(ツ)_/¯
|
I would think a lot more methods needs to be static, but maybe they already are? Except from that 👍 |
|
Yeah, created an issue to track the |
|
Could you also rename following two filenames according to the naming convention? It always bugs me.
Other than that + what @cmelchior pointed, you have a 👍 from my heart. Also, I'd ask @kneth's review as well. |
| return myClass; | ||
| } | ||
|
|
||
| void jprint(JNIEnv *env, char *txt) |
There was a problem hiding this comment.
This has been useful for debugging in the past. But I assume we have another way now?
There was a problem hiding this comment.
no idea, didn't spend time to check. No one is calling them so instead of making them static, I just removed them. They would be easily back by checking git history if needed.
There was a problem hiding this comment.
We are using the TR macros for logging. It will make it easier to support non-Android.
|
👍 |
5 participants
functions are defined with a proper declaration. JNI function not
found problem can only be seen at run time, thus we need to do so.