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

Retry timestamp write to swift #89

Closed
wants to merge 2 commits into from
Closed

Retry timestamp write to swift #89

wants to merge 2 commits into from

Conversation

SuperSandro2000
Copy link
Member

No description provided.

Copy link
Contributor

@majewsky majewsky left a comment

Choose a reason for hiding this comment

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

Please explain via comment what the loop is doing and why.

Copy link
Contributor

@majewsky majewsky left a comment

Choose a reason for hiding this comment

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

I apologize for not looking at this better the first time around. There are two main problems with this implementation:

  1. On persistent error, the for-loop never terminates because errs is scoped to the loop and thus always reset to empty on each loop iteration. It will never contain more than one entry. errs needs to be allocated outside of the loop.
  2. When finally failing, there is a useless sleep before that. The sleep should happen only after checking if the loop is to be terminated.

While fixing this, I refactored the retry logic into a separate function to make it stand out better, and then it just ended up so different from the original that I put it up as a separate PR -> #90

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