feat(jins-meme): add support for jins meme smart glasses #1212

lora-reames · 2017-03-22T03:14:12Z

https://jins-meme.com/en/
https://jins-meme.github.io/sdkdoc/en/

@PatrickR

(no-unused-variable) @ionic-native/plugins/jins-meme/index.ts[1, 10]: Unused import: 'Injectable' stream.js:74 throw er; // Unhandled stream error in pipe. ^ Error: Failed to lint: @ionic-native/plugins/jins-meme/index.ts [1, 10]: Unused import: 'Injectable'.

as per #1210

Jins meme

Feature jins meme

Revert "Feature jins meme"

merge joshbabb/ionic-native into blyncsync/ionic-native

rgbsuede · 2017-03-22T03:15:53Z

Please squash this into one commit with message:

feat(jins-meme): add support for jins meme smart glasses

If/when merging :)

ihadeed · 2017-03-22T05:20:42Z

src/@ionic-native/plugins/jins-meme/index.ts

+ * ```
+ * import { JinsMeme } from 'ionic-native';
+ *
+ * JinsMeme.functionName('Hello', 123)


Please replace this usage block with actual usage example.

Also, I notice you're using the old template from Ionic Native 2.x.

Usage should look like:

import { JinsMeme } from '@ionic-native/jins-meme'; constructor(private jinsMeme: JinsMeme) { } ... this.jinsMeme.functionName() .then(...) .catch(...);

See how other plugins are documented.

ihadeed · 2017-03-22T05:21:05Z

src/@ionic-native/plugins/jins-meme/index.ts

+ *
+ * @usage
+ * ```
+ * import { JinsMeme } from 'ionic-native';


Package name should be @ionic-native/jins-meme

ihadeed · 2017-03-22T05:21:43Z

src/@ionic-native/plugins/jins-meme/index.ts

+declare var cordova: any;
+
+/**
+ * @name JinsMeme


Name should be Jins Meme with a space between the two words

ihadeed · 2017-03-22T05:21:59Z

src/@ionic-native/plugins/jins-meme/index.ts

+  plugin: 'JinsMemeSDK-Plugin-Cordova', // npm package name, example: cordova-plugin-camera
+  pluginRef: 'cordova.plugins.JinsMemePlugin', // the variable reference to call the plugin, example: navigator.geolocation
+  repo: 'https://github.com/jins-meme/JinsMemeSDK-Plugin-Cordova.git', // the github repository URL for the plugin
+  install: '' // OPTIONAL install command, in case the plugin requires variables


Remove this line since there is no custom install command needed

ihadeed · 2017-03-22T05:22:12Z

src/@ionic-native/plugins/jins-meme/index.ts

+ */
+@Plugin({
+  pluginName: 'JinsMeme',
+  plugin: 'JinsMemeSDK-Plugin-Cordova', // npm package name, example: cordova-plugin-camera


Remove the documentation beside this line, and the following 3 lines.

ihadeed · 2017-03-22T05:24:00Z

src/@ionic-native/plugins/jins-meme/index.ts

+  @Cordova()
+  static disconnect(): Promise<any> { return; }
+
+  /**


Remove white-space before this comment block, apply to other occurrences as well.

ihadeed · 2017-03-22T05:24:11Z

src/@ionic-native/plugins/jins-meme/index.ts

+  *@returns {Promise<any>}
+  */
+  @Cordova()
+  static setAppClientID(appClientId: string, clientSecret: string): Promise<any> { return; }


All methods should not be static. Please remove the static keyword from all methods below.

ihadeed · 2017-03-22T05:27:38Z

DEVELOPER.md

@@ -110,7 +110,7 @@ The `@Cordova` decorator has a few more options now.

 ### Testing your changes

-You need to run `npm run build` in the `ionic-native` project, this will create a `dist` directory. Then, you must go to your ionic application folder and replace your current `node_modules/ionic-native/dist/` with the newly generated one.
+You need to run `npm run build` in the `ionic-native` project, this will create a `dist/packages-dist` directory. Then, you must go to your ionic application folder and replace your current `node_modules/@ionic-native/` with the newly generated `dist/packages-dist/@ionic-native/` folder.


I prefer if this commit is removed from the PR.

Ideally a PR should be to modify/add one thing.

ihadeed · 2017-03-22T05:28:42Z

src/@ionic-native/plugins/jins-meme/index.ts

+  * @param {string} target
+  */
+  // @Cordova({observable: true})
+  static connect(target: string): Observable<any> {


Add the following decorator for this method:

@CordovaCheck({ observable: true })

This will ensure that the plugin is installed and available before attempting to run your custom code. CordovaCheck should be imported from @ionic-native/core

ihadeed · 2017-03-22T05:29:32Z

src/@ionic-native/plugins/jins-meme/index.ts

+  */
+  // @Cordova({observable: true})
+  static connect(target: string): Observable<any> {
+     return new Observable<any> ((observer: any) => {


Remove white-space between <any> and ((observer...

ihadeed · 2017-03-22T05:31:03Z

Thanks for submitting a PR.

Please review the changes requested.

Also, please run npm run lint or gulp lint to review and fix any code style issues.

see #1212 (review)

rgbsuede

As of BlyncSync@e86b462 I have made all of these changes!

lora-reames and others added 11 commits March 21, 2017 11:58

(feat) add support for JinsMemeSDK-Plugin-Cordova

fae1d4c

(fix) correct jins-meme plugin name

62ca259

#feature/jins-meme initial commit

f94a362

thanks @PatrickR, @jins-meme, @tjpeden

import fixes

873ee71

(no-unused-variable) @ionic-native/plugins/jins-meme/index.ts[1, 10]: Unused import: 'Injectable' stream.js:74 throw er; // Unhandled stream error in pipe. ^ Error: Failed to lint: @ionic-native/plugins/jins-meme/index.ts [1, 10]: Unused import: 'Injectable'.

update developer guide

fd3b8d0

as per #1210

Merge pull request #1 from BlyncSync/jins-meme

0486fb0

Jins meme

removed unused import

9fb40c0

Merge pull request #1 from joshbabb/feature/jins-meme

156d43d

Feature jins meme

Revert "Feature jins meme"

d4d05be

Merge pull request #2 from joshbabb/revert-1-feature/jins-meme

5cc9c0f

Revert "Feature jins meme"

Merge pull request #2 from joshbabb/master

0b40073

merge joshbabb/ionic-native into blyncsync/ionic-native

lora-reames changed the title ~~(feat) add support for jins meme smart glasses~~ feat(jins-meme): add support for jins meme smart glasses Mar 22, 2017

This was referenced Mar 22, 2017

feat(jins-meme): add support for jins meme smart glasses #1211

Closed

DEVELOPER.md up to date? #1210

Closed

Josh Babb added 2 commits March 21, 2017 22:27

correct developer docs (#3)

484e7a1

remove unused import and update documentation

80b7238

ihadeed requested changes Mar 22, 2017

View reviewed changes

joshbabb added 3 commits March 22, 2017 01:08

ihadeed's requested changed

09bc6cf

see #1212 (review)

usage update

e41a278

revert DEVELOPER.md to limit scope of PR

e86b462

rgbsuede approved these changes Mar 22, 2017

View reviewed changes

ihadeed merged commit 9c88cfb into danielsogl:master Mar 22, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(jins-meme): add support for jins meme smart glasses #1212

feat(jins-meme): add support for jins meme smart glasses #1212

lora-reames commented Mar 22, 2017 •

edited

Loading

rgbsuede commented Mar 22, 2017

ihadeed Mar 22, 2017

ihadeed Mar 22, 2017

ihadeed Mar 22, 2017

ihadeed Mar 22, 2017

ihadeed Mar 22, 2017

ihadeed Mar 22, 2017

ihadeed Mar 22, 2017

ihadeed Mar 22, 2017

ihadeed Mar 22, 2017

ihadeed Mar 22, 2017

ihadeed commented Mar 22, 2017

rgbsuede left a comment

feat(jins-meme): add support for jins meme smart glasses #1212

feat(jins-meme): add support for jins meme smart glasses #1212

Conversation

lora-reames commented Mar 22, 2017 • edited Loading

rgbsuede commented Mar 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ihadeed commented Mar 22, 2017

rgbsuede left a comment

Choose a reason for hiding this comment

lora-reames commented Mar 22, 2017 •

edited

Loading