Add article "Ensure Plugins Quality" #824
Conversation
angeltsvetkov force-pushed the atsvetkov/plugin-sanity-check branch 2 times, most recently from 8cced80 to f224c75 Compare 
plugins/plugin-sanity-check.md Outdated
| - built for iOS | ||
| - bundled with webpack | ||
| | ||
| Ignoring any of these non-functional requirements could lead to not working app. Let’s call verification of these requirements – sanity checks. |
There was a problem hiding this comment.
Grammatical - change to could lead to an app not working
Also grammatical/logical - Throughout the article we'll be referring to the verification of those requirements as 'sanity checks'
There was a problem hiding this comment.
Can't get the first one. Is this really correct?
Agree on the second one.
plugins/plugin-sanity-check.md Outdated
| | ||
| Ignoring any of these non-functional requirements could lead to not working app. Let’s call verification of these requirements – sanity checks. | ||
| | ||
| This article answers the following questions: |
There was a problem hiding this comment.
Change to This article will cover:
and proceed with statements instead of questions. For example: What you need to setup sanity checks for your NS plugins; Check your plugin's code for readability.... The (lack of a) question mark makes all the difference.
There was a problem hiding this comment.
Prerequisites might also be shorter for What you need ...
plugins/plugin-sanity-check.md Outdated
| | ||
| ## What do you need to setup sanity checks for your NativeScript plugin? | ||
| | ||
| As we mentioned earlier, NativeScript plugins are building blocks. They work as part of the NativeScript application. |
There was a problem hiding this comment.
.. as part of Nativescript application**s**.
plugins/plugin-sanity-check.md Outdated
| ## What do you need to setup sanity checks for your NativeScript plugin? | ||
| | ||
| As we mentioned earlier, NativeScript plugins are building blocks. They work as part of the NativeScript application. | ||
| Add a simple `demo` application that shows how the plugin works. If the plugin is Angular compatible, there should be another `demo-angular` application as well. |
There was a problem hiding this comment.
This sentence doesn't sound logically related to the previous one - As we mentioned earlier... Consider adding something along the lines of NativeScript apps can be written in both Angular, or just plain JavaScript/TypeScript, they can also take advantage of the webpack feature to cut off the excess code. In order to ensure that your/our plugin runs reliably in any NativeScript application, there are certain prerequisites you may need to complete.
plugins/plugin-sanity-check.md Outdated
| "prepublishOnly": "npm run build" | ||
| ``` | ||
| | ||
| This script will be executed before the package is prepared and packed, only on npm publish. More details can be found in the [npm-script documentation](https://docs.npmjs.com/misc/scripts). The build script (defined above) will be executed. This is important because in this way the TypeScript of the plugin will be compiled and the required metadata will be generated any time before publishing. |
There was a problem hiding this comment.
This script will be executed before the package is prepared and packed, only on npm publish. is shortly followed by The build script (defined above) will be executed. Which statement is correct, and when?
also (grammatical) ... will be generated any time ... replace with ... will be generated every time ...
plugins/plugin-sanity-check.md Outdated
| | ||
| ## How to check your plugin’s code for readability, maintainability, and functionality errors? | ||
| | ||
| [TSLint](https://palantir.github.io/tslint/) is a great tool for static analysis of your plugin’s code. It will test the plugin for readability and maintainability as well as functionality errors based on customizable rules. This is a [list of all TSLint rules](https://palantir.github.io/tslint/rules/) you can check for. |
There was a problem hiding this comment.
(Logical) change the last sentence to A complete list with the available TSLint rules can be found in the [tsling repository](https://palantir.github.io/tslint/rules/)
plugins/plugin-sanity-check.md Outdated
| | ||
| `tools` and `platform-tools` components define that the latest revision of Android SDK Tools will be installed. More info [here](https://docs.travis-ci.com/user/languages/android/#Overview) | ||
| | ||
| `build-tools-23.0.1` component defines the BuildTools version that will be used. |
There was a problem hiding this comment.
we recommend building with build-tools 25.0.2
There was a problem hiding this comment.
Should clarify this! My understanding was that {N} works great with the latest versions of Android and iOS but I think the right approach is to test the apps and plugins with the most popular versions of Android and iOS and as you can see from the statistics for Android it is not 25.
There was a problem hiding this comment.
I assume you are confusing build-tools with compileSdk, or possibly even with targetSdk. {N} works equally well with all compileSdks. And we do also recommend building against the latest compileSdk, we do it implicitly too, unless stated otherwise while tns build-ing with the --compileSdk flag.
As for the build-tools - it is a set of libraries and executables that processes resources, packages your application, signs your app package, etc. They are required to build android applications.
The build-tools requirement is also in our setup articles - http://uatdocs.nativescript.org/start/ns-setup-os-x#system-requirements
There was a problem hiding this comment.
I get it. Thanks for the info.
plugins/plugin-sanity-check.md Outdated
| | ||
| `extra-android-m2repository` component defines the support library repositories. | ||
| | ||
| `sys-img-armeabi-v7a-android-23` component defines the system image. |
There was a problem hiding this comment.
component defines the system image of the android emulator.
plugins/plugin-sanity-check.md Outdated
| ``` | ||
| The scripts (`ci.android.build` and `ci.ios.build`) that are executed for building for iOS and Android are located in [package.json](https://github.com/NativeScript/nativescript-facebook/blob/master/demo/package.json#L48) file of any of the demo apps. | ||
| | ||
| If everything is configured appropriately these sanity checks will be executed on every code change. The result will look like this: |
There was a problem hiding this comment.
If everything is configured appropriately these sanity checks will be executed on every code change. The result will look like this: -> If everything is configured properly, the sanity checks will execute on every code change. The result, and whether the checks pass or not, will look like this:
plugins/plugin-sanity-check.md Outdated
| | ||
|  | ||
| | ||
| The huge benefit for NativeScript plugin authors is that once having this sanity checks set up, You guys can be more confident about the quality of your plugins. |
There was a problem hiding this comment.
Since you are addressing plugin authors directly, you can talk about their plugins. The huge benefit for NativeScript plugin authors is that once having this sanity checks set up, You guys can be more confident about the quality of your plugins. -> The main benefit of having sanity checks in place for your NativeScript plugins is that you can develop without spending additional time to ensure your changes don't break existing applications depending on your plugin.
plugins/plugin-sanity-check.md Outdated
| | ||
| The huge benefit for NativeScript plugin authors is that once having this sanity checks set up, You guys can be more confident about the quality of your plugins. | ||
| | ||
| By the way, do not forget to [add TravisCI badge](https://docs.travis-ci.com/user/status-images/) in your NativeScript plugin's project! It's fancy! No newline at end of file |
There was a problem hiding this comment.
By the way, do not forget to [add TravisCI badge](https://docs.travis-ci.com/user/status-images/) in your NativeScript plugin's project! It's fancy! -> Do not forget to .... plugin repository/project/readme. It reports live status of your CI build and makes your plugin look more reliable.
There was a problem hiding this comment.
Overall I think this is great. My comments are mostly minor grammatical things, although I did have a few relatively trivial things I thought could use some clarifications.
If you have any questions let me know, although I will be out next Monday & Tuesday for a US holiday 🇺🇸 🎆
plugins/plugin-sanity-check.md Outdated
| @@ -0,0 +1,291 @@ | |||
| --- | |||
| title: Sanity Check A Plugin | |||
| description: Guideline how to sanity check NativeScript plugin | |||
There was a problem hiding this comment.
A series of tips on how to sanity check a NativeScript plugin
plugins/plugin-sanity-check.md Outdated
| --- | ||
| title: Sanity Check A Plugin | ||
| description: Guideline how to sanity check NativeScript plugin | ||
| position: 60 |
There was a problem hiding this comment.
This is the same position as the “Plugin Reference” article. I think we’d want this article to come before that one... but I could go either way. In any case make sure you alter these numbers before you publish. I’d probably just change the position in plugin-reference.md to something like 100 because I think we’ll always want that one last.
plugins/plugin-sanity-check.md Outdated
| - built for iOS | ||
| - bundled with webpack | ||
| | ||
| Ignoring any of these non-functional requirements could lead to not working app. Throughout the article we'll be referring to the verification of those requirements as 'sanity checks'. |
There was a problem hiding this comment.
could lead to not working app
could lead to an app that doesn’t work as expected
There was a problem hiding this comment.
Throughout the article
Throughout this article,
plugins/plugin-sanity-check.md Outdated
| Ignoring any of these non-functional requirements could lead to not working app. Throughout the article we'll be referring to the verification of those requirements as 'sanity checks'. | ||
| | ||
| This article will cover: | ||
| - [Setup sanity checks for your NativeScript plugin](#setup-sanity-checks-for-your-nativescript-plugin) |
There was a problem hiding this comment.
Setup sanity checks
Setting up sanity checks
plugins/plugin-sanity-check.md Outdated
| | ||
| This article will cover: | ||
| - [Setup sanity checks for your NativeScript plugin](#setup-sanity-checks-for-your-nativescript-plugin) | ||
| - [Check your plugin’s code for readability, maintainability, and functionality errors](#check-your-plugins-code-for-readability-maintainability-and-functionality-errors) |
plugins/plugin-sanity-check.md Outdated
| ``` | ||
| - npm install -g nativescript | ||
| - npm install -g tslint | ||
| - npm install -g typescript@2.2.2 |
There was a problem hiding this comment.
Does it have to be TypeScript version 2.2.2? Seems like that might become a maintenance nightmare for this documentation.
plugins/plugin-sanity-check.md Outdated
| ``` | ||
| - tns usage-reporting disable | ||
| ``` | ||
| Configures anonymous usage reporting for the NativeScript CLI. More info [here](https://github.com/NativeScript/nativescript-cli/blob/master/docs/man_pages/general/usage-reporting.md). |
There was a problem hiding this comment.
I think you mean error reporting here....? You’re including the same text twice.
plugins/plugin-sanity-check.md Outdated
| ``` | ||
| Refer to nativescript-facebook [.travis.yml file](https://github.com/NativeScript/nativescript-facebook/blob/doc/.travis.yml#L61-L67) to see this in reality. | ||
| | ||
| As we mention earlier the plugin should be sanity checked on Android as well as on iOS. The Android specific requirements can be defined in `.travis.yml` file in `android` section: |
There was a problem hiding this comment.
As we mentioned earlier,
plugins/plugin-sanity-check.md Outdated
| before_install: nvm install 6.10.3 | ||
| script: cd demo && npm run build.plugin && npm i && npm run build-android-bundle && cd ../demo-angular && npm run build.plugin && npm i && npm run build-android-bundle | ||
| ``` | ||
| This stage includes 2 builds that run in parallel. One for Android and one for iOS. Note that the nodejs on the Linux machine is installed because it is not included in the image. |
There was a problem hiding this comment.
This stage includes two builds that run in parallel—one for Android, and one for iOS.
plugins/plugin-sanity-check.md Outdated
| | ||
| The main benefit of having sanity checks in place for your NativeScript plugins is that you can develop without spending additional time to ensure your changes don't break existing applications depending on your plugin. | ||
| | ||
| Do not forget to [add TravisCI badge](https://docs.travis-ci.com/user/status-images/) in your NativeScript plugin's project! It reports live status of your CI build and makes your plugin look more reliable. No newline at end of file |
There was a problem hiding this comment.
to add a Travis CI badge
| position: 50 | ||
| slug: sanity-check-plugin | ||
| --- | ||
| |
There was a problem hiding this comment.
I would suggest to add your article title here as h1.
plugins/plugin-sanity-check.md Outdated
| | ||
| NativeScript apps can be written in both Angular, or just plain JavaScript/TypeScript and they can also take advantage of the [webpack bundling]({% slug bundling-with-webpack %}) to cut off the excess code. In order to ensure that your plugin runs reliably in any NativeScript application, there are certain prerequisites you may need to complete. | ||
| | ||
| All plugins should have a demo folder that contains a demo application showing how the plugin works. If you use the NativeScript plugin seed you will have this folder by default. If your plugin is a user interface plugin, and you need to test the plugin in both Angular and non-Angular apps, you should have an additional demo-angular folder containing an Angular app you can test your plugin in. Refer to the article ["Supporting Angular in UI Plugins"]({% slug supporting-angular-in-ui-plugins %}) for more details. |
There was a problem hiding this comment.
You can link the plugin seed when you mention it here.
plugins/plugin-sanity-check.md Outdated
| | ||
| [TSLint](https://palantir.github.io/tslint/) is a great tool for static analysis of your plugin’s code. It will test the plugin for readability and maintainability as well as functionality errors based on customizable rules. A complete list with the available TSLint rules can be found in the [tsling repository](https://palantir.github.io/tslint/rules/). | ||
| | ||
| The official NativeScript plugin seed recommends TSLint rules defined in this [tslint.json](https://github.com/NativeScript/nativescript-plugin-seed/blob/master/tslint.json) file. |
plugins/plugin-sanity-check.md Outdated
| ``` | ||
| | ||
| This script executes the `tslint` command passing the tslint rules defined in `tslint.json` file. The installed `node_modules` will be excluded from the static analysis. | ||
| Having `tslint.json` on root level allows using same TSLint rules for demo apps as well adding the same script. |
There was a problem hiding this comment.
"Having tslint.json on root level allows using same TSLint rules for demo apps as well adding the same script." -> @tjvantoll can correct this better but I would say it like "Having tslint.json on root level allows using the same TSLint rules for both demo apps by adding the same script."
plugins/plugin-sanity-check.md Outdated
| | ||
| The NativeScript command for building Android and iOS apps is: | ||
| | ||
| `tns build Android` and `tns build ios` |
There was a problem hiding this comment.
Probably the casing doesn't matter but for integrity I would suggest to use "android" instead of "Android"
plugins/plugin-sanity-check.md Outdated
| ``` | ||
| Refer to nativescript-facebook [.travis.yml file](https://github.com/NativeScript/nativescript-facebook/blob/doc/.travis.yml#L60-L62) to see this in reality. | ||
| | ||
| As we mention earlier, the plugin should be sanity checked on Android as well as on iOS. The Android specific requirements can be defined in `.travis.yml` file in `android` section: |
plugins/plugin-sanity-check.md Outdated
| before_install: nvm install 6.10.3 | ||
| script: cd demo && npm run build.plugin && npm i && npm run build-android-bundle && cd ../demo-angular && npm run build.plugin && npm i && npm run build-android-bundle | ||
| ``` | ||
| This stage includes two builds that run in parallel—one for Android, and one for iOS. Note that the nodejs on the Linux machine is installed because it is not included in the image. |
There was a problem hiding this comment.
Add spaces before and after the dash.
plugins/plugin-sanity-check.md Outdated
| jdk: oraclejdk8 | ||
| script: cd demo && npm run ci.ios.build && cd ../demo-angular && npm run ci.ios.build | ||
| ``` | ||
| The scripts (`ci.android.build` and `ci.ios.build`) that are executed for building for iOS and Android are located in [package.json](https://github.com/NativeScript/nativescript-facebook/blob/master/demo/package.json#L49) file of any of the demo apps. |
There was a problem hiding this comment.
You can escape the repetition of "for" -> .. executed to build for iOS and Android..
| | ||
| The main benefit of having sanity checks in place for your NativeScript plugins is that you can develop without spending additional time to ensure your changes don't break existing applications depending on your plugin. | ||
| | ||
| Do not forget to [add a Travis CI badge](https://docs.travis-ci.com/user/status-images/) in your NativeScript plugin's project! It reports live status of your CI build and makes your plugin look more reliable. No newline at end of file |
There was a problem hiding this comment.
I would add ".. in github" -> Do not forget to add a Travis CI badge in your NativeScript plugin's project in github.
There was a problem hiding this comment.
Since there is no other place I think it is common sense that we are talking about GitHub repo.
| | ||
| Travis CI is a great way to automate plugin’s sanity checks. It is free for open-source projects. More details can be found in [Travis CI documentation](https://docs.travis-ci.com/). Travis CI will boot a virtual machine and execute commands based on the provided configuration in your `.travis.yml` file. | ||
| | ||
| First things first! Add an empty `.travis.yml` file on the root level of your plugin. |
There was a problem hiding this comment.
I think somewhere between these row you can add a sentence saying that if you use the seed, you have an initial .travis.yml file setup.
angeltsvetkov force-pushed the atsvetkov/plugin-sanity-check branch 2 times, most recently from 4182d02 to e1d53ed Compare 
angeltsvetkov force-pushed the atsvetkov/plugin-sanity-check branch 2 times, most recently from 557c9d0 to e6654fa Compare angeltsvetkov force-pushed the atsvetkov/plugin-sanity-check branch from e6654fa to 0b6b3a8 Compare angeltsvetkov changed the title 
| This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
4 participants
No description provided.