Enable retryQueue by default.

segment-integrations / analytics.js-integration-segmentio

The Segmentio analytics.js integration.

MIT License

8 stars 28 forks source link

Enable retryQueue by default. #49

Closed f2prateek closed 6 years ago

f2prateek commented 6 years ago

Now that we've graduated the feature out of beta, we should enable this by default for all customers.

Existing customers will have this flag set to false (LIB-608), so that they don't notice a change in behaviour after this lands.

f2prateek commented 6 years ago

There's a risk here that we are not testing the code path where retryQueue = false diligently anymore. However given that we plan to phase the non-retryQueue branch out, I think this should be ok.

Two other ideas:

set retryQueue in existing tests and only rely on retryQueue = true in https://github.com/segment-integrations/analytics.js-integration-segmentio/blob/master/test/index.test.js#L987
Copy all existing tests and and run them with retryQueue = true

f2prateek commented 6 years ago

Pinging a couple folks from destinations to get some more eyes on this.

f2prateek commented 6 years ago

I updated the tests to be a bit more thorough! To make sure we test both cases well, I moved the existing tests into a table driven test. There is some redundancy with the e2e tests as a result, but I wanted to avoid changing too much logic.

f2prateek commented 6 years ago

@fathyb could you take a second look with the updated tests and give a 👍/👎 ?