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

Adding preset for leapmotion controller #1056

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

seem-less
Copy link

No description provided.

Copy link
Member

@junlarsen junlarsen left a comment

Choose a reason for hiding this comment

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

Looks good so far. Just a few nits and a couple things to add:

  1. JavaCPP is built and distributed in CI, meaning there should be a new .github/workflows/leapmotion.yml workflow. You can copy one of the existing workflows and go from there. It's usually just a matter of changing a few names.
    • Because the user has to download leapmotion themselves, we also need to do this in CI. You should download leapmotion in the action files found in .github/actions for the platforms your preset is available for.
    • Testing this is easily done by re-targeting the workflow's branch against your own fork.
  2. The license for leapmotion should be copied into the preset's directory.

Comment on lines +28 to +69
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>${javacpp.moduleId}</artifactId>
<version>${project.version}</version>
<classifier>${javacpp.platform.android-arm}</classifier>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>${javacpp.moduleId}</artifactId>
<version>${project.version}</version>
<classifier>${javacpp.platform.android-arm64}</classifier>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>${javacpp.moduleId}</artifactId>
<version>${project.version}</version>
<classifier>${javacpp.platform.android-x86}</classifier>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>${javacpp.moduleId}</artifactId>
<version>${project.version}</version>
<classifier>${javacpp.platform.android-x86_64}</classifier>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>${javacpp.moduleId}</artifactId>
<version>${project.version}</version>
<classifier>${javacpp.platform.linux-x86}</classifier>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>${javacpp.moduleId}</artifactId>
<version>${project.version}</version>
<classifier>${javacpp.platform.linux-x86_64}</classifier>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>${javacpp.moduleId}</artifactId>
<version>${project.version}</version>
<classifier>${javacpp.platform.macosx-x86_64}</classifier>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

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

Judging from the current cppbuild.sh we're currently building windows-x86_64 and windows-x86. The other platforms which are not supported yet should be removed from the dependency list.

Comment on lines +139 to +145
requires static org.bytedeco.${javacpp.moduleId}.android.arm;
requires static org.bytedeco.${javacpp.moduleId}.android.arm64;
requires static org.bytedeco.${javacpp.moduleId}.android.x86;
requires static org.bytedeco.${javacpp.moduleId}.android.x86_64;
requires static org.bytedeco.${javacpp.moduleId}.linux.x86;
requires static org.bytedeco.${javacpp.moduleId}.linux.x86_64;
requires static org.bytedeco.${javacpp.moduleId}.macosx.x86_64;
Copy link
Member

Choose a reason for hiding this comment

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

Similarly to how we're only adding dependencies for existing platforms, we only want the available platforms listed here. These entries should be removed until more platforms are added.

===============================

[![Gitter](https://badges.gitter.im/bytedeco/javacpp.svg)](https://gitter.im/bytedeco/javacpp) [![Maven Central](https://maven-badges.herokuapp.com/maven-central/org.bytedeco/leapmotion/badge.svg)](https://maven-badges.herokuapp.com/maven-central/org.bytedeco/leapmotion) [![Sonatype Nexus (Snapshots)](https://img.shields.io/nexus/s/https/oss.sonatype.org/org.bytedeco/leapmotion.svg)](http://bytedeco.org/builds/)
<sup>Build status for all platforms:</sup> [![leapmotion](https://github.com/bytedeco/javacpp-presets/workflows/leapmotion/badge.svg)](https://github.com/bytedeco/javacpp-presets/actions?query=workflow%3Aleapmotion) <sup>Commercial support:</sup> [![xscode](https://img.shields.io/badge/Available%20on-xs%3Acode-blue?style=?style=plastic&logo=appveyor&logo=data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAEAAAABACAMAAACdt4HsAAAAGXRFWHRTb2Z0d2FyZQBBZG9iZSBJbWFnZVJlYWR5ccllPAAAAAZQTFRF////////VXz1bAAAAAJ0Uk5T/wDltzBKAAAAlUlEQVR42uzXSwqAMAwE0Mn9L+3Ggtgkk35QwcnSJo9S+yGwM9DCooCbgn4YrJ4CIPUcQF7/XSBbx2TEz4sAZ2q1RAECBAiYBlCtvwN+KiYAlG7UDGj59MViT9hOwEqAhYCtAsUZvL6I6W8c2wcbd+LIWSCHSTeSAAECngN4xxIDSK9f4B9t377Wd7H5Nt7/Xz8eAgwAvesLRjYYPuUAAAAASUVORK5CYII=)](https://xscode.com/bytedeco/javacpp-presets)
Copy link
Member

Choose a reason for hiding this comment

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

We should also add the workflow status badge to the main README.md as well as listing leapmotion as one of the available presets.

@@ -630,6 +630,7 @@
<module>cpu_features</module>
<module>modsecurity</module>
<module>systems</module>
<module>leapmotion</module>
Copy link
Member

Choose a reason for hiding this comment

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

Leapmotion should also be registered as an available module for the platforms you're building. Each JavaCPP platform has its own profile in this pom and leapmotion should be registered in these.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants