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

Conditionally download updates. #171

Merged
merged 2 commits into from
Jun 2, 2016
Merged

Conditionally download updates. #171

merged 2 commits into from
Jun 2, 2016

Conversation

joshaber
Copy link
Contributor

@joshaber joshaber commented Jun 1, 2016

Fixes #111

@joshaber
Copy link
Contributor Author

joshaber commented Jun 1, 2016

This assumes we can trust servers to return and respect etags. Is that a reasonable assumption these days?

@joshaber
Copy link
Contributor Author

joshaber commented Jun 1, 2016

Looks like S3 respects etags which I gotta think is where most updates live.

@@ -40,6 +40,10 @@ @interface SQRLUpdater ()

@property (atomic, readwrite) SQRLUpdaterState state;

/// The etag of the currently downloaded update, nil if no update has been
/// downloaded.
@property (atomic, copy) NSString *etag;
Copy link
Member

@keithduncan keithduncan Jun 2, 2016

Choose a reason for hiding this comment

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

Does this need to be atomic, inside the scope of the RACCommand shouldn’t access be serialised?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LOL I honestly couldn't recall anymore, so I went with safe over sorry.

@keithduncan
Copy link
Member

I don’t think we lose anything with the atomic and I don’t want to split hairs, 👍 to merge this

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.

2 participants