-
Notifications
You must be signed in to change notification settings - Fork 16
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
misc(syncer): make timeout configurable #162
misc(syncer): make timeout configurable #162
Conversation
@vgonkivs This one's ready for review :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong. The State struct is not supplied to the Syncer, but the Syncer provides it's cuttent state to user through State method on Syncer. Therefore, you can't configure anything through it in Syncer.
Instead, we should add this configuration field to the Config
I see. I'm sorry for this error, I'll make the changes asap! |
hey @AryanGodara, thanks for you contribution. Let me help you to improve your code: If you take a look here, you will see the Parameter struct that allows configuring the syncer. In the syncer c-tor we create default parameters and override them using |
Wow thank you for explaining it in such detail @vgonkivs :D |
Hi @AryanGodara thank you so much for your contribution. We've decided that we don't actually need this feature unfortunately. I apologise for the last minute change of mind. There are many other good first issues that we welcome you to work on, however! |
It's quite alright @renaynay :D |
…ean up `Head` func a bit (#163) This PR renames a poorly named metric from `unrecent_header` to `outdated_head` which is more descriptive / accurate. It also cleans up a bit of the code inside of `sync_head.go`. This PR is meant to override #162 if we agree that the timeout should not be configurable (see [comment](#155 (comment))
Overview
Resolves #155
Checklist