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

Implement ping manager #1

Merged
merged 9 commits into from
Jul 19, 2024
Merged

Conversation

muzzammilshahid
Copy link
Member

No description provided.

@muzzammilshahid muzzammilshahid requested a review from om26er July 13, 2024 11:13
@muzzammilshahid muzzammilshahid force-pushed the ping-manager branch 2 times, most recently from 33ea51e to e3df152 Compare July 13, 2024 13:12
@om26er
Copy link
Member

om26er commented Jul 18, 2024

Thinking a bit more about it. We need to remove the use of interface all together.

The design of this library should only be to allow calling functions at specific time. What happens in those functions is the decision of the developers using the library.

Instead of using Client we should just use id int64 in Add/Remove/Reset functions. Doing that should make this library fully generic

@om26er
Copy link
Member

om26er commented Jul 18, 2024

This naturally means that any reference to the word "Pinger" in this library becomes wrong

workerpool.go Outdated

type wpConfig struct {
interval time.Duration
dataFunc func() []byte
Copy link
Member

@om26er om26er Jul 19, 2024

Choose a reason for hiding this comment

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

Suggested change
dataFunc func() []byte
callback func() error

workerpool.go Outdated

type wpConfig struct {
interval time.Duration
callback func() []byte
Copy link
Member

@om26er om26er Jul 19, 2024

Choose a reason for hiding this comment

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

Suggested change
callback func() []byte
callback func() error

@muzzammilshahid muzzammilshahid force-pushed the ping-manager branch 2 times, most recently from 5140cf1 to fd32693 Compare July 19, 2024 09:15
@muzzammilshahid muzzammilshahid requested a review from om26er July 19, 2024 09:17
@muzzammilshahid muzzammilshahid merged commit c306fb2 into xconnio:main Jul 19, 2024
1 check passed
@muzzammilshahid muzzammilshahid deleted the ping-manager branch July 19, 2024 09:30
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