Skip to content
This repository was archived by the owner on Mar 20, 2018. It is now read-only.

Fix bundling bug - #114

Merged
geigerj merged 1 commit into
googleapis:masterfrom
geigerj:master
Jun 29, 2016
Merged

geigerj merged 1 commit into
googleapis:masterfrom
geigerj:master

Conversation

@geigerj

@geigerj geigerj commented Jun 28, 2016

Copy link
Copy Markdown
Contributor

Previously, one timer was associated with each Executor. This timer
was not reset when it fired, which meant that only a single request
could ever be triggered by a delay threshold.

This change switches to using one timer per bundle. This correctly
allows different bundles to fire under different timers.

cc: @tbetbetbe @jmuk
Fixes #113

@geigerj

geigerj commented Jun 28, 2016

Copy link
Copy Markdown
Contributor Author

Per offline discussion, I will investigate about adding more thorough tests for delay threshold-triggered bundling as a follow-up to this PR. See #115

@codecov-io

codecov-io commented Jun 28, 2016 •

Copy link
Copy Markdown

Current coverage is 97.16%

Merging #114 into master will not change coverage

@@             master       #114   diff @@
==========================================
  Files             8          8          
  Lines           599        599          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits            582        582          
  Misses           17         17          
  Partials          0          0          

Powered by Codecov. Last updated by a5e1944...d766e10

@tbetbetbe

Copy link
Copy Markdown
Contributor

LGTM

Previously, one timer was associated with each Executor. This timer
was not reset when it fired, which meant that only a single request
could ever be triggered by a delay threshold.

This change switches to using one timer per bundle. This correctly
allows different bundles to fire under different timers.
@bjwatson

Copy link
Copy Markdown
Contributor

LGTM

@geigerj
geigerj merged commit 75fc5bd into googleapis:master Jun 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants