Skip to content
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

Formatting: JNI module for Java/Android wrappers #420

Merged
merged 6 commits into from
Mar 11, 2019

Conversation

forelocked-beobachter
Copy link
Contributor

Add JNI to the formatting target. It is a bit special because we have to specify path to JDK headers (and actually install JDK for that).

clang-tidy suggested some changes, they are applied as a separate commit for review:

  • Avoid returning from both "then" and "else" branches
  • Initialize possibly uninitialized variables

Also, it turned out that clang-format treats private and public as C++ keywords when formatting C code which results in dumb formatting. Rename identifiers into private_key to avoid this.

ilammy and others added 5 commits March 7, 2019 14:17
Add JNI to the formatting target. It is a bit special because we have to
specify path to JVM's headers.
- Avoid returning from both "then" and "else" branches
- Initialize possibly uninitialized variables
clang-format treats them as C++ keywords when formatting code which
results in dumb formatting so don't use these words.
Keep it in CircleCI configuration for now, we'll move it into Docker
image later.
@forelocked-beobachter forelocked-beobachter added O-Android 🤖 Operating system: Android W-JavaThemis ☕ Wrapper: Java, Java and Kotlin API labels Mar 7, 2019
Copy link
Contributor

@vixentael vixentael left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

Separate (huge) thank you for splitting all the things into separate commits, it was much easier to read.

@ilammy ilammy merged commit 2d419d1 into cossacklabs:master Mar 11, 2019
@ilammy ilammy deleted the format-jni branch March 11, 2019 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-Android 🤖 Operating system: Android W-JavaThemis ☕ Wrapper: Java, Java and Kotlin API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants