Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-20604] Refactor iOS module #41

Merged
merged 16 commits into from
Feb 28, 2018
Merged

[TIMOB-20604] Refactor iOS module #41

merged 16 commits into from
Feb 28, 2018

Conversation

hansemannn
Copy link
Contributor

@hansemannn hansemannn commented Dec 14, 2017

@hansemannn hansemannn added this to the v2.0.2 milestone Dec 15, 2017
@hansemannn hansemannn self-assigned this Dec 15, 2017
@@ -44,7 +44,7 @@ description: |
for the [enterregions](Modules.Geofence.enterregions) or [exitregions](Modules.Geofence.exitregions) events, respectively.
For example:

Geofence.addEventListener("enterregion", function(e) {
Geofence.addEventListener('enterregion', function(e) {

Choose a reason for hiding this comment

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

Please change 'enterregion' to 'enterregions' as 'enterregions' is the event name.

@@ -44,7 +44,7 @@ description: |
for the [enterregions](Modules.Geofence.enterregions) or [exitregions](Modules.Geofence.exitregions) events, respectively.
For example:

Geofence.addEventListener("enterregion", function(e) {
Geofence.addEventListener('enterregions', function(e) {

Choose a reason for hiding this comment

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

  1. Before start monitoring region, we have request location permission. This is not mentioned in document. I have mentioned with example in comment of the ticket. Can you add same in document ?
  2. Please update examples mentioned in module. There are iOS7 related stuff there. And need to add location permission.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Examples updated!

.travis.yml Outdated
env:
global:
- "MODULE_NAME=ti.geofence"
- TRAVIS_NODE_VERSION="4"
- TRAVIS_NODE_VERSION="8"

Choose a reason for hiding this comment

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

I am seeing jerkin is not able to build this and giving error -
"ERROR: appc requires Node.js >=8.0.0"
Is TRAVIS_NODE_VERSION is cause of this error?

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 need to remove Travis completely, which is now done.

@hansemannn
Copy link
Contributor Author

@vijaysingh-axway @garymathews Also updated the play-services related docs that haven't been updated when including the ti.playservices dependency

Copy link

@vijaysingh-axway vijaysingh-axway left a comment

Choose a reason for hiding this comment

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

CR passed.

@ewieberappc
Copy link
Contributor

FR Passed. Location is correctly tracked and I receive enter and exit location events while monitoring regions. Tested using the provided samples as well as the geolocation/geofence suite

@ewieberappc ewieberappc merged commit 9c0ec2c into master Feb 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants