Skip to content
This repository was archived by the owner on May 20, 2025. It is now read-only.

Query update with current package#3

Merged
geof90 merged 13 commits intocheck-update-and-downloadfrom
query-update-with-current-package
Aug 22, 2015
Merged

Query update with current package#3
geof90 merged 13 commits intocheck-update-and-downloadfrom
query-update-with-current-package

Conversation

@geof90
Copy link
Contributor

@geof90 geof90 commented Aug 13, 2015

Edited the code to

  • Save current package info to a file on installing a new bundle from the server
  • Get current package info from disk and send to server for comparison when querying for updates

Copy link
Contributor

Choose a reason for hiding this comment

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

put spacing between if and opening paranthesis, similarly for else and braces.. that is our coding guidelines.
feedback throughout the code.

If someone has a link to the coding guideline page, that would be great for Geoffery.

Copy link
Contributor

Choose a reason for hiding this comment

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

Reduce redundancy by combining logic;

var defaultPackage = {appVersion: configuration.appVersion};
if (!err && localPackage !== null && localPackage.appVersion === configuration.appVersion) {
defaultPackage = localPackage;
} else if (err) {
console.log(err);
}

sdk.queryUpdateWithCurrentPackage(defaultPackage, callback);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

I should have mentioned this before, final change:

if (!err && localPackage && localPackage...)

checking this way verifies localPackage against null and undefined.. dont need an explicit null check.

@shishirx34
Copy link
Contributor

LGTM!

You should wait on @itsananderson to signoff for the objective C changes..
Obj C looks good to me, but Will would be in a better state to identify issues with it.

@itsananderson
Copy link

The file Examples/HybridMobileDeployCompanion/react-packager-cache-5737493143262339187995942fd693ba should not be included.

@itsananderson
Copy link

Sorry for the delayed review. LGTM once you remove the temp file.

@geof90 geof90 merged this pull request into check-update-and-download Aug 22, 2015
@geof90 geof90 deleted the query-update-with-current-package branch August 22, 2015 00:00
max-mironov pushed a commit to max-mironov/react-native-code-push that referenced this pull request May 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants