feat(zeroconf): add cordova-plugin-zeroconf support #1260
Conversation
|
|
||
| /** | ||
| * Publishes a new service. | ||
| * @usage |
There was a problem hiding this comment.
Move all the @usage documentation to the main @usage block in the class documentation.
The usage here will not be rendered in the docs page.
Please apply this fix to all methods below. Simply cut & paste them above.
There was a problem hiding this comment.
I copied this style from the BLE plugin, and you're right, this isn't rendered in the docs page, too.
Do I guess this should be changed for at least the BLE plugin also, and probably other plugins are affected, too.
There was a problem hiding this comment.
Yes this should be changed in the BLE plugin and any other similar occurrences. All usage should stay in the main @usage block in the class documentation.
Thanks!
| import { Cordova, Plugin } from '@ionic-native/core'; | ||
| import { Observable } from 'rxjs/Observable'; | ||
|
|
||
| export interface ZeroconfService { |
There was a problem hiding this comment.
Rename this to ZeroConfService
There was a problem hiding this comment.
Sorry, forget about the changes regarding the capitalization of the name.
I assumed ZeroConf is a brand name since the plugin repo has it written in that way. I traced this thing down to this page: http://www.zeroconf.org/ and realized it's called Zeroconf.
I will delete the other comments below.
There was a problem hiding this comment.
Yes, it's complicated ;-)
Thanks, I really think it is easier for people who actually know/use zeroconf that way, even if "ZeroConf" looks more natural first!
And I don't want to call this plugin @ionic-native/zero-conf (which would have to change, too, I guess) because that looks just funny...
There was a problem hiding this comment.
Yes you're right.
FYI, in cases where we have brand names that need to be capitalized. We capitalize the class name but keep the package name as one word. You can see this in OneSignal, PayPal, SQLite .. etc
There was a problem hiding this comment.
Ah, okay. Thanks, I missed that - should have looked more carefully!
This PR adds support for cordova-plugin-zeroconf, as requested in #636.
Please note that I only tested the browsing/watching functionality, since I've never published a zeroconf service myself (on a mobile device) and I wouldn't really know how to test that.
A note on naming: The plugin uses all lowercase
zeroconfas pluginRef and in examples, but uses camel-caseZeroConfinternally and sporadically in the docs. However, the official zeroconf site refers toZeroconfwith initial caps only. I decided to go with the latter, even if that may look unfamiliar to some (but hey, so doesGeolocation). It also avoid naming the packagezero-confwhich isn't used widely, and might cause even more confusion.Yes, sometimes I do pay attention to such details ;-)