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-15267] Exposed versionName and VersionCode #5145

Merged
merged 6 commits into from Mar 7, 2014

Conversation

salachi
Copy link
Contributor

@salachi salachi commented Dec 18, 2013

return appVersionName;
}

private static void initializeVersionValues() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you trying putting the content of this method to TiPlatformHelper.initialize() ? That way you don't need to do null checks in the getter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this doesn't look like a frequently used property, I thought on-demand initialization is better as we don't take the performance penalty during app initialization.

@ghost ghost assigned hieupham007 Jan 6, 2014
@hieupham007
Copy link
Contributor

Please add documentation for the new APIs.

@@ -205,6 +205,16 @@ public String getRuntime()
return KrollRuntime.getInstance().getRuntimeName();
}

@Kroll.getProperty @Kroll.method
public int getAppVersionCode() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these Android only APIs? If that's the case, they should be implement in ../modules/titanium/platform/AndroidModule.java.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, these are Android specific APIs, I will move to AndroidModule.

@salachi
Copy link
Contributor Author

salachi commented Feb 4, 2014

Took care of review comments

summary: |
The version number of the application.
type: Number
since: 3.2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

3.3.0

@hieupham007
Copy link
Contributor

Please also add a test case in the ticket.

@hieupham007
Copy link
Contributor

Code reviewed. Please address comments

@vishalduggal
Copy link
Contributor

This code must not be in the Ti.Media.Android but rather in Ti.App since these are app specific properties.

Code Reviewed. REJECTED

- name: appVersionName
summary: |
The version name of the application.
type: Number
Copy link
Contributor

Choose a reason for hiding this comment

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

String

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

@salachi
Copy link
Contributor Author

salachi commented Mar 7, 2014

Addressed review comments

@vishalduggal
Copy link
Contributor

CR + FR Passed. APPROVED

vishalduggal added a commit that referenced this pull request Mar 7, 2014
[TIMOB-15267] Exposed versionName and VersionCode
@vishalduggal vishalduggal merged commit 93adf69 into tidev:master Mar 7, 2014
farfromrefug pushed a commit to Akylas/titanium_mobile that referenced this pull request Aug 15, 2014
[TIMOB-15267] Exposed versionName and VersionCode
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.

None yet

3 participants