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.

Sign up for GitHub

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

Jump to bottom

[TIMOB-26133] TiAPI: Make Ti.Geolocation.hasCompass a method #10121

Closed
wants to merge 5 commits into from
Closed

[TIMOB-26133] TiAPI: Make Ti.Geolocation.hasCompass a method #10121

wants to merge 5 commits into from

Conversation

hansemannn
Copy link
Collaborator

@hansemannn hansemannn added this to the 8.0.0 milestone Jun 19, 2018
@build
Copy link
Contributor

build commented Jun 19, 2018 •
edited

Fails
🚫

Tests have failed, see below for more information.
Messages
📖

💾 Here's the generated SDK zipfile.

Tests:

Classname Name Time Error
android.Titanium.Geolocation hasCompass 0.001 TypeError: Ti.Geolocation.hasCompass is not a function

Generated by 🚫 dangerJS

hansemannn
hansemannn commented Jun 20, 2018
View reviewed changes
android/modules/geolocation/src/java/ti/modules/titanium/geolocation/GeolocationModule.java
*/
// clang-format off
@Kroll.method
public boolean hasCompass()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@garymathews Question here: Could it be that V8 does not allow this because the above method (getHasCompass) also represents a property that is hasCompass and therefore blocks this method declaration?

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically, yes. They're both registering as "hasCompass" on the JS type. One as a property that is backed by getHasCompass() and again as a function. I believe V8 is throwing out the duplicate definition.

I don't see how a property with the exact same name can be changed from a "typical" boolean property to a method property in a non-breaking way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mhh okay. In that case we just leave the API as it is

@hansemannn hansemannn closed this Jun 21, 2018
@hansemannn hansemannn deleted the TIMOB-26133 branch June 21, 2018 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgtcoolguy sgtcoolguy sgtcoolguy left review comments

@vijaysingh-axway vijaysingh-axway Awaiting requested review from vijaysingh-axway

Assignees
No one assigned
Labels
android docs ios
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hansemannn @build @sgtcoolguy