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.

Sign up for GitHub

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

Jump to bottom

Keeping timers relative to start of tunnel initiation is confusing #309

Open
agrover opened this issue Aug 24, 2022 · 5 comments
Open

Keeping timers relative to start of tunnel initiation is confusing #309

agrover opened this issue Aug 24, 2022 · 5 comments
Labels
code quality A general code quality issue with the code, that is not a bug good first issue Good for newcomers

Comments

@agrover
Copy link
Collaborator

agrover commented Aug 24, 2022

Keeping timers this way means that for example update_session_timers takes a time_now param that's a Duration instead of an Instant. All entries in Tunn::timers are Durations when many of them are "the time that something happened" and idiomatically I think these would be better expressed as Instants.
@agrover agrover added good first issue Good for newcomers code quality A general code quality issue with the code, that is not a bug labels Aug 24, 2022
@Noah-Kennedy
Copy link
Collaborator

Agreed.

@crajcan crajcan mentioned this issue Oct 27, 2022
Open
@crajcan
Copy link

crajcan commented Nov 5, 2022 •
edited
Loading

@agrover @Noah-Kennedy Anyone available to review? 😁

@thomaseizinger
Copy link
Contributor

Friendly ping! This would be a nice addition :)

@crajcan
Copy link

crajcan commented Jan 24, 2024

Friendly ping! This would be a nice addition :)

@thomaseizinger does my PR accomplish what you're looking for? If so I'll rebase it.

@thomaseizinger
Copy link
Contributor

Friendly ping! This would be a nice addition :)

@thomaseizinger does my PR accomplish what you're looking for? If so I'll rebase it.

Yes! I am hoping that we can also move this to be entirely SANS-IO so we have control over the what "now" is :)

See #391.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
code quality A general code quality issue with the code, that is not a bug good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@agrover @thomaseizinger @Noah-Kennedy @crajcan