search

dbaelz / Konclik

Konclik: Kotlin/Native Command Line Interface Kit
Apache License 2.0
46 stars 4 forks source link

Update to new MPP plugin #3

Closed DrewCarlson closed 5 years ago

DrewCarlson commented 5 years ago

Additionally the following changes were made to the Example module

Publishing: I dropped the bintray publishing plugin and reconfigured the project to make use of the publications generated by the new MPP plugin. Note that all the artifacts need to be published under the same bintray package. Please see my repo to inspect the published package. You can publish to bintray with ./gradlew publish

dbaelz commented 5 years ago

Hi, thanks for your contribution. I was only able to get a rough overview on the pull request. Tomorrow I'm taking a closer look into the code and add some comments when required.

As a first step: Could you fix the Travis CI build for your branch?

DrewCarlson commented 5 years ago

🚀 All green! Sorry about that.

I also updated the deploy command that travis uses to the publish task, note that for this to work your bintray repo needs a single package named Konclik.

Edit: Side note: if you're interested, I've had success with Appveyor as a Windows CI host for some of my repos (example). Happy to add the necessary config if you're interested in turning that on.

DrewCarlson commented 5 years ago

I created a new Repo and Package for Konclik (now with upper K). Could you take a look and tell me if I'm done the setup correctly?

Looks good, the publish task should put everything in that package.

The README is outdated after this PR is merged to develop. But that's no problem, I'll fix it later one with a new PR. Feel free to add information and corrections to this PR.

I'll add some info regarding consuming the lib, running tests, ect. to the README and let you take it from there.

We could add windows librarys for bintray with the experimental Travis CI windows image. That's a good ideas. Either with this PR or another one. As you like it.

Haven't tried Windows with Travis but it would definitely be nice to have everything on one CI service. I'll look into that as well.

Out of curiosity: Do you think the JS target will be used in any way to create CLI applications? I'm not sure about that, but it's good to support this target, too.

My reasons for the Js target were: Interacting with Js framework tooling to build CLI utilities for Kotlin/JS applications, leveraging popular Js packages to build more robust CLI apps (high level stuff like chalk), and to enable building tools for web-developers with the ability to distribute it in a way they are familiar with. I'm actually using this target already to build internal tooling that leverages existing Js deployment code. It may be worth renaming the target Nodejs, thoughts?

DrewCarlson commented 5 years ago

@dbaelz I was able to get windows tests passing on Travis, but for some reason the build stalls after the tests pass(see here). I've left the windows config commented out, it may be easier for you to debug this or I can look into it later.

dbaelz commented 5 years ago

@dbaelz I was able to get windows tests passing on Travis, but for some reason the build stalls after the tests pass(see here). I've left the windows config commented out, it may be easier for you to debug this or I can look into it later.

That's fine for me. I prefer to merge the PR as soon as possible and we could improve the Travis CI windows build later.

Some answers to your previous post

I'll add some info regarding consuming the lib, running tests, ect. to the README and let you take it from there.

A good start for me. I'll expand them in another PR. Then I can also contribute something meaningful to the project. :grin:

My reasons for the Js target were: Interacting with Js framework tooling to build CLI utilities for Kotlin/JS applications, leveraging popular Js packages to build more robust CLI apps (high level stuff like chalk), and to enable building tools for web-developers with the ability to distribute it in a way they are familiar with. I'm actually using this target already to build internal tooling that leverages existing Js deployment code. It may be worth renaming the target Nodejs, thoughts?

It's great to know that the project is used somewhere else. The target name is fine for me, even if it's usually used with nodejs.

Next steps

In my option, this PR is ready for merge. IDE support and Travis CI windows support should be done in another PR. Do you have any open points? Otherwise I'll merge.

DrewCarlson commented 5 years ago

👍 We'll stick with the js target name and if you want to open issues for IDE support and Windows CI, we can discuss further there 😃.

Agreed, this PR looks good to merge! 💥