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

Attempted fix for initial load issue. #47

Conversation

aedificator-nl
Copy link
Contributor

@aedificator-nl aedificator-nl commented Jun 8, 2020

All parts of the guard statement have been split into let statements and two separate if statements.

If localStoredData == nil we still want to synchronize.

See issue #48.

All parts of the guard statement have been split into let statements and two separate if statements.

If localStoredData == nil we still want to synchronize.
return
}

if (localStoredDate != nil && remoteStoredDate != nil && !(remoteStoredDate!.timeIntervalSince1970 > localStoredDate!.timeIntervalSince1970)) {
Copy link
Owner

Choose a reason for hiding this comment

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

these are force-unwrapped. Mind addressing that? Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely. Have added extra check and avoid force-unwrapping.

Copy link
Owner

Choose a reason for hiding this comment

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

localStoredDate! is still force unwrapped :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, damn typos... Anyway, now it should actually be fixed ;)

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks - will check it out this weekend

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 have force-unwrapping here again, see comment below.

@ArtSabintsev ArtSabintsev self-requested a review June 10, 2020 10:05
@ArtSabintsev ArtSabintsev linked an issue Jun 10, 2020 that may be closed by this pull request
@ArtSabintsev
Copy link
Owner

ArtSabintsev commented Jun 12, 2020

This does not compile in Xcode 11.5. A couple errors around optionals.

@aedificator-nl
Copy link
Contributor Author

The force-unwrapping of the optionals is necessary to avoid Xcode throwing a fit. Given that a nil check has already been done the code that force-unwraps is not reachable with a nil value, thus never throwing an error.

To me this seems the best way to use the optional values here. Do you have a suggestion for a better way to approach this? Thanks in advance!

@ArtSabintsev
Copy link
Owner

I'll fix the force-unwrap issue once I compile and test your code in a branch of mine. Thanks!

@aedificator-nl
Copy link
Contributor Author

Thanks! And thanks again for the whole project, awesome piece of code :)

@ArtSabintsev
Copy link
Owner

Thank you! I appreciate that!

@ArtSabintsev
Copy link
Owner

Alright, tested it in a person project - worked well. Will merge your stuff then overwrite with a cleaner implementation that has no force-uwnraps. Thanks again!

@ArtSabintsev ArtSabintsev merged commit 30d6e01 into ArtSabintsev:master Jun 15, 2020
@ArtSabintsev
Copy link
Owner

Here's the release notes: https://github.com/ArtSabintsev/Zephyr/releases/tag/3.6.0

It should be available across all dependency managers now.

@aedificator-nl
Copy link
Contributor Author

No problem, glad to be able to help. Thank you for the kind mention in the release notes!

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.

Issue with initial load from iCloud
2 participants