Subject: [quicwg/base-drafts] Fix Insert Count from 4 to 3 (#4923)

From: Daiki AMINAKA <[email-masked]>
Date: Fri, 02 Jul 2021 21:39:32 -0700

The Insert Count should be 3 at this point (before inserting duplicated header)
It should become 4 after this duplication

Is my understanding correct?
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/4923

-- Commit Summary --

  * Fix Insert Count from 4 to 3

-- File Changes --

    M draft-ietf-quic-qpack.md (2)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/4923.patch
https://github.com/quicwg/base-drafts/pull/4923.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/4923

---

From: Martin Thomson <[email-masked]>
Date: Sun, 04 Jul 2021 23:10:09 -0700

@martinthomson commented on this pull request.

I don't believe that this is correct.  The table has three insertions before this point, so the new count is 4 (this is consistent with the "Required Insert Count = 4" that follows in the example.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/4923#pullrequestreview-698823601

---

From: Daiki AMINAKA <[email-masked]>
Date: Mon, 05 Jul 2021 22:01:08 -0700

I deduced my correction from dumped dynamic table.
This duplicate instruction looks trying to duplicate header of :authority=www.example.com as shown in dumped dynamic table.
This absolute index is 0 as I corrected.
The original absolute index of 1 is pointing to :path=/sample/path, looks wrong.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/4923#issuecomment-874460874

---

From: Martin Thomson <[email-masked]>
Date: Mon, 05 Jul 2021 23:33:31 -0700

@martinthomson approved this pull request.

Oh, I see my mistake.  Yes, this is right: this is the calculation for the absolute index, which is indeed 0 (and not 1).  Thanks for explaining.  This needs to use the insert count *before* the duplicated entry is added.  The rest of the example is correct.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/4923#pullrequestreview-699579585

---

From: Daiki AMINAKA <[email-masked]>
Date: Tue, 06 Jul 2021 23:51:59 -0700

Thanks for confirmation!
why CI is failing?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/4923#issuecomment-875336504

---

From: Martin Thomson <[email-masked]>
Date: Wed, 07 Jul 2021 00:13:09 -0700

Not your fault.  The change is valid, but there is a system error (I've corrected it, so I might rerun the build to clear the problem).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/4923#issuecomment-875349600

---

From: afrind <[email-masked]>
Date: Wed, 07 Jul 2021 11:31:44 -0700

Thanks for catching this.  Can you resubmit to kick the ci?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/4923#issuecomment-875834574

---

From: Daiki AMINAKA <[email-masked]>
Date: Wed, 07 Jul 2021 12:37:13 -0700

@afrind You mean create new PR?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/4923#issuecomment-875877706

---

From: Mike Bishop <[email-masked]>
Date: Wed, 07 Jul 2021 12:49:06 -0700

@ami-GS, the simplest version is probably this, assuming that `origin` points to this repo:

    git fetch origin
    git merge origin/main
    git push

If you've used another name to point to this repo, like `upstream`, sub that in the first two lines.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/4923#issuecomment-875884711

---

From: Daiki AMINAKA <[email-masked]>
Date: Wed, 07 Jul 2021 12:56:57 -0700

@ami-GS pushed 1 commit.

7beef7ae77693b6265ca97a42d4c61da282c4405  Fix Insert Count from 4 to 3

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4923/files/249f55643588679a918a50a3c7cc81fc9af71d2a..7beef7ae77693b6265ca97a42d4c61da282c4405

---

From: Daiki AMINAKA <[email-masked]>
Date: Wed, 07 Jul 2021 12:57:43 -0700

@MikeBishop rebase then force pushed, thanks!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/4923#issuecomment-875890034

---

From: afrind <[email-masked]>
Date: Wed, 07 Jul 2021 13:50:31 -0700

Merged #4923 into main.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/4923#event-4991396468