-
Notifications
You must be signed in to change notification settings - Fork 120
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
Add command line interface #1198
Conversation
f2b97c6
to
be42f0c
Compare
905c44c
to
c0cb3ff
Compare
This is great @kmadsen - I just gave it a try, no issues. A few random suggestions, in no particular order, some of them could be tracked as tailwork:
|
Yea thanks for the reminder! removes the TODO because they already solved the problems I was dealing with. it's all updated
Yep agree but as tailwork 👍 ; was planning on making that another change so it can receive separate code review
Tailwork agree 👍 ; Would like to dig in to see if we can make it more automated than documented |
42601e3
to
334da52
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @kmadsen
Left a minor comment (not blocking the PR).
Another follow up would be creating a Gradle plugin that runs the MapboxJavaCli
program.
Also don't forget to cut tickets with the tailwork listed in #1198 (comment)
334da52
to
61cc5a8
Compare
implementation("commons-cli:commons-cli:1.4") | ||
|
||
// Kotlin | ||
implementation("org.jetbrains.kotlin:kotlin-stdlib-jdk8") | ||
|
||
// Testing | ||
testImplementation("org.jetbrains.kotlin:kotlin-test-junit") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should move these to gradle/dependencies.gradle
#1198 (comment)
Resolves https://github.com/mapbox/navigation-sdks/issues/542
Usage
Alternatives
The original ticket asks to test json on any previous build of mapbox-java, which is a little complicated with gradle - so here was an alternative to make the CLI in NodeJs #1189 - This approach was closed because we would rather build it in kotlin.
Next steps
Distributing
We will want to add this jar to our mapbox-java releases as an asset. Notice how classyshark does this too. This task is to make it so directions api can use them in their CI process.
Features
Currently you can make directions api calls with
make
commands usingcurl
. This project will allow us to move those commands to use the actual mapbox-java implementation (it's not implemented yet, but this project is set up to hopefully make it easy to do so)Feel free to add your ideas! This is also written in kotlin, so it will be easier to start building our mapbox-java modules in kotlin.