Skip to content

Conversation

@radeva
Copy link
Contributor

@radeva radeva commented Feb 19, 2018

  • update readme with more info about the forums and slack
  • add PR template
  • add CONTRIBUTING.md and DevelopmentWorkflow.md

- update readme with more info about the forums and slack
- add PR template
- add CONTRIBUTING.md and DevelopmentWorkflow.md
@ghost ghost assigned radeva Feb 19, 2018
@ghost ghost added the new PR label Feb 19, 2018

3. Build the app for Android or iOS
```bash
tns run android/ios
Copy link
Contributor

Choose a reason for hiding this comment

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

if only building, then command could be changed to

tns build android
# or
ths build ios

Someone might try running tns run android/ios and get The input is not valid sub-command for 'run' command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is true. I will change the commands, let's still discuss if build or run should be used.


## PR Checklist

- [ ] The PR title follows our guidelines: https://github.com/NativeScript/NativeScript/blob/master/CONTRIBUTING.md#commit-messages.

Choose a reason for hiding this comment

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

the PR title or the commit messages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, it's the title and the body of the message. The rules for the commit message are valid for the PR as well. We can think about how to say it better.


### Prerequisites

* Install your native toolchain and NativeScript as [described in the docs](https://docs.nativescript.org/plugins/plugins)

Choose a reason for hiding this comment

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

is this link correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, it is. It links to "Using Plugins" section.

### Install dependencies

```
$ cd nativescript-imagepicker/src

Choose a reason for hiding this comment

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

npm run demo.android will make npm i for you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is true. I wanted to separate the actions you do on the plugin and the ones you do on the demo to make it more visible to the user. We can discuss which is better.

$ npm run demo.android
```

### Run the demo-angular app

Choose a reason for hiding this comment

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

why running the Angular app should be different from running the TS app?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't have npm commands for running the demo app. We can think of explaining how to the run a demo app from its folder only.

## Get Help
Please, use [github issues](https://github.com/NativeScript/nativescript-imagepicker/issues) strictly for [reporting bugs](CONTRIBUTING.md#reporting-bugs) or [requesting features](CONTRIBUTING.md#requesting-new-features). For general questions and support, check out the [NativeScript community forum](https://discourse.nativescript.org/) or ask our experts in [NativeScript community Slack channel](http://developer.telerik.com/wp-login.php?action=slack-invitation).
![](https://ga-beacon.appspot.com/UA-111455-24/nativescript/nativescript-imagepicker?pixel)

Choose a reason for hiding this comment

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

what's this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is some tracking that exists across all main repos. I added it here for completeness, but if nobody is reviewing the stats, probably we can remove it.

@radeva radeva merged commit f405298 into master Feb 20, 2018
@ghost ghost removed the new PR label Feb 20, 2018
@radeva radeva deleted the radeva/update-readme branch February 20, 2018 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants