ice-wg / draft-ice-pac

Other
0 stars 0 forks source link

Name the timer 'Tpac' #17

Closed cdh4u closed 5 years ago

cdh4u commented 5 years ago

Fixes #16

cdh4u commented 5 years ago

Note that 'Ta' in 8445 is the interval, not the timer. It uses 'timer Ta' to refer to the actual timer.

You are right, eventhoug 8445 in some places do say "whenever Ta fires". I'll update the PR.

I'm not sure this change is an improvement, but open to considering it.

I think it is much easier to talk about "timer Tpac" than "the timer that does this and that". For example, during the ICEbis work, we often talked about "timer Ta".

It may not sound like a big differnce at this point, when we are still working on the draft, but I think it will make a difference in the future when/if people start referring to the timer.

juberti commented 5 years ago

@pthatcherg, @akeranen, wdyt?

pthatcherg commented 5 years ago

I don't think we need this, but if we want to name the timer, I would suggest:

  1. Call it "Tpac" and not "Timer Tpac"

  2. Not change so much of the existing text. Maybe put in the name (for future conversations) and just replace the once place where we say "the aforementioned timer".

cdh4u commented 5 years ago

I don't think we need this, but if we want to name the timer, I would suggest:

  1. Call it "Tpac" and not "Timer Tpac"

I initially did that, but Justin pointed out that, when comparing to Ta, Ta is just the interval used by a timer.

  1. Not change so much of the existing text. Maybe put in the name (for future conversations) and > just replace the once place where we say "the aforementioned timer".

I think that, when we talk about about a timer, we should use the name of the timer.

pthatcherg commented 5 years ago

The first mentions of timers in 8445 says "The gathering process is controlled using a timer, Ta. Every time Ta expires" but then later is says "The generation of ordinary and triggered connectivity checks is governed by timer Ta". So it looks like 8445 isn't self consistent.

Meanwhile, we call the RTO timer "timer RTO", so we don't need the "Tfoo" model. How about we just call it "timer PAC", or better yet "the PAC timer"?

cdh4u commented 5 years ago

The first mentions of timers in 8445 says "The gathering process is controlled using a timer, Ta. Every time Ta expires" but then later is says "The generation of ordinary and triggered connectivity > checks is governed by timer Ta". So it looks like 8445 isn't self consistent.

I left that part out on purpose :)

The Terminology section uses "Timer Ta", though.

Meanwhile, we call the RTO timer "timer RTO", so we don't need the "Tfoo" model. How about we just call it "timer PAC", or better yet "the PAC timer"?

I want to have a timer name. As I indicated to Justin when we discussed this, it will make future discussions much easier, when we refer to the timer. It will also make life easier for implementers.

What about using "timer Tpac" in the first occurrence, and then "Tpac"?

BTW, if you are concerned about the number of changes in the PR, note that we have already merged a set of PRs, so the new version of the draft will contain changes from the current version no matter what.

pthatcherg commented 5 years ago

I don't see any reason to use the T prefix when always say "timer" along with it. It's redundant. I'd prefer "PAC timer or "timer PAC".

But I don't care enough to keep talking about it. I'd rather just pick something and move on.

cdh4u commented 5 years ago

I don't see any reason to use the T prefix when always say "timer" along with it. It's redundant. I'd prefer "PAC timer or "timer PAC".

But I don't care enough to keep talking about it. I'd rather just pick something and move on.

I have no problem with "timer PAC" if you prefer that. All I want is a timer name that people can be refer to.

akeranen commented 5 years ago

I think it's very useful to have a name for the timer and both Tpac and PAC timer work for me.

cdh4u commented 5 years ago

I will merge the pull request. There will still be time (during WGLC, IESG review) to fine tune the timer name, if there is a need to.

juberti commented 5 years ago

There’s still more to do on the PR, but sounds like we have a path forward.

On Tue, Jul 23, 2019 at 3:19 PM cdh4u notifications@github.com wrote:

I will merge the pull request. There will still be time (during WGLC, IESG review) to fine tune the timer name, if there is a need to.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/ice-wg/draft-ice-pac/pull/17?email_source=notifications&email_token=AAN4X7M65NPIFZNEVZUA6VTQA5KVPA5CNFSM4IFLHNKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2UFAUA#issuecomment-514347088, or mute the thread https://github.com/notifications/unsubscribe-auth/AAN4X7ONCDX54F45KTDHSHLQA5KVPANCNFSM4IFLHNKA .

cdh4u commented 5 years ago

There’s still more to do on the PR, but sounds like we have a path forward.

Ok, so I will not merge yet.

juberti commented 5 years ago

Updated the PR to use 'PAC timer', and only where necessary. I'm happy with this if you are.

cdh4u commented 5 years ago

Updated the PR to use 'PAC timer', and only where necessary. I'm happy with this if you are.

Looks good.