Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings
This repository was archived by the owner on Nov 24, 2023. It is now read-only.

Paho MQTT library #18

Merged
bashor merged 7 commits into Kotlin:master from simonegiacomelli:master
Mar 6, 2018
Merged

Paho MQTT library #18

bashor merged 7 commits into Kotlin:master from simonegiacomelli:master
Mar 6, 2018

Conversation

@simonegiacomelli
Copy link
Contributor

@simonegiacomelli simonegiacomelli commented Feb 23, 2018

These are an incomplete but working header for Paho MQTT javascript library by Eclipse.

I followed this advice
https://discuss.kotlinlang.org/t/kotlin-headers-for-mqtt-javascript-library/6726/5

@bashor bashor self-requested a review February 26, 2018 15:43
@bashor bashor self-assigned this Feb 26, 2018
@bashor bashor requested review from anton-bannykh and removed request for anton-bannykh February 26, 2018 15:44
@@ -0,0 +1,3 @@
{
"version": "3.1.1"
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to rename the directory v0 according this version, e.g. to v3

}

object PahoTest {
fun tryit(conf:dynamic) {
Copy link
Member

Choose a reason for hiding this comment

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

Minor: missed space before dynamic

}
}

object PahoTest {
Copy link
Member

Choose a reason for hiding this comment

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

API test should be moved to separate directory, see how it's done jquery

//headers for Paho JavaScript Client
//The Paho JavaScript Client is an MQTT browser-based client library written in Javascript that uses WebSockets to connect to an MQTT Broker.
//https://github.com/eclipse/paho.mqtt.javascript

Copy link
Member

Choose a reason for hiding this comment

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

Please add information about yourself, like:

// Contributed by: name/nick <link to github user or your email>

@@ -0,0 +1,63 @@
//headers for Paho JavaScript Client
Copy link
Member

Choose a reason for hiding this comment

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

Minor: missed spaces after //


package js.externals.paho_mqtt

external object Paho {
Copy link
Member

Choose a reason for hiding this comment

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

Have you tried to convert https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/paho-mqtt/ using ts2kt?

Copy link
Contributor Author

@simonegiacomelli simonegiacomelli Mar 3, 2018

Choose a reason for hiding this comment

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

I tried ts2kt on paho-mqtt typescript but I got an error.
I tried it on jquery typescript and everything was fine... so I think my proceedings were all right.

This is the error I got
ts2kt version: 0.1.3
Converting index.d.ts
fs.js:640
return binding.open(pathModule._makeLong(path), stringToFlags(flags), mode);
^

Error: ENOENT: no such file or directory, open 'C:\module.d.ts'
at Error (native)
at Object.fs.openSync (fs.js:640:18)
at Object.fs.readFileSync (fs.js:508:33)
at getScriptSnapshotFromFile (C:\Users\Simone\AppData\Roaming\npm\node_modules\ts2kt\ts2kt.js:5398:46)
at translate (C:\Users\Simone\AppData\Roaming\npm\node_modules\ts2kt\ts2kt.js:5623:21)
at translateToDir (C:\Users\Simone\AppData\Roaming\npm\node_modules\ts2kt\ts2kt.js:5177:26)
at main (C:\Users\Simone\AppData\Roaming\npm\node_modules\ts2kt\ts2kt.js:5387:5)
at C:\Users\Simone\AppData\Roaming\npm\node_modules\ts2kt\ts2kt.js:6243:3
at Object. (C:\Users\Simone\AppData\Roaming\npm\node_modules\ts2kt\ts2kt.js:6246:2)
at Module._compile (module.js:570:32)

Copy link
Member

Choose a reason for hiding this comment

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

external object Paho {
object MQTT {
class Client(hostname: String, port: Int, clientId: String) {
val hostname: String
Copy link
Member

Choose a reason for hiding this comment

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

According the docs it should be called host.

var destinationName: String
}

class Response {
Copy link
Member

Choose a reason for hiding this comment

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

I think it should be an interface

val errorMessage: String
}

interface Options {
Copy link
Member

Choose a reason for hiding this comment

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

IMHO ConnectOptions is a better name for this, also looks like most of properties can be optional

Copy link
Member

bashor commented Feb 26, 2018

@simonegiacomelli, thank you for the contribution! (You are first ;))

Would be nice to get some improvements (see comments) before we merge it.

simonegiacomelli reacted with thumbs up emoji

Copy link
Contributor Author

simonegiacomelli commented Mar 3, 2018
edited
Loading

@bashor I fixed all your requests except the optional parameters of ConnectOptions

Copy link
Member

bashor commented Mar 6, 2018

Nice! Thank you, @simonegiacomelli!

@bashor bashor merged commit 3483c81 into Kotlin:master Mar 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@bashor bashor bashor requested changes

Reviewers whose approvals may not affect merge requirements

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /