Alex Wolfe alexkwolfe

@alexkwolfe

@kcaponeAP, did you read Chris' comment about the lead.email field?

@alexkwolfe
Number type should gracefully deal with NaN
@alexkwolfe

LGTM

@alexkwolfe

As noted by Chris in slack, all production uses of list_items have been changed to values.

@alexkwolfe

LGTM

@alexkwolfe

LGTM

@alexkwolfe

It looks to me like this should be failing.

@alexkwolfe

Issue #15 claims that LMB provides "yes" and "no" for the values in the Veteran field, but this test uses "Y" and "N". Which is correct?

@alexkwolfe

What happened to this comment?

@alexkwolfe

The LeadCost value is a string, but the test is verifying that it is an integer. Why?

@alexkwolfe

Is this still required?

@alexkwolfe

Can this be removed?

@alexkwolfe

The boolean type used by the lead handler takes care of parsing true or false based on common string values, so it might be worth removing the pars…

@alexkwolfe

You can use the assert.isNull function here.

@alexkwolfe

It's not clear to me what problem issue #14 is reporting...

@alexkwolfe

It is not necessary to URL encode JSON.

@alexkwolfe

@cgrayson, never mind. I see now.

@alexkwolfe

@cgrayson, in your comment you said "In other deprecation cases we're continuing to set both the old and the new, but in this case that's impossibl…

@alexkwolfe

This routine to find the latest claim isn't necessary. The claim API returns the claim being made. If the event were passed to this function instea…

@alexkwolfe

@ros3bud, is this in progress?

@alexkwolfe

Other than that one comment, LGTM.

@alexkwolfe

Is it worth it to use claim.created_at here rather than Date.now()?

@alexkwolfe
  • @alexkwolfe cff994a
    The deregister function now deregisters an integration
@alexkwolfe

I think you want to keep this line, no? The operator should just be changed to -=, isn't that right?

@alexkwolfe

This test was meant to ensure that the age does not include the time on page. I'm looking at this on my phone so it's hard to be sure, but it appea…

@alexkwolfe

LGTM

@alexkwolfe
@alexkwolfe
Query integration should validate that a list_id and values are set
@alexkwolfe
  • @alexkwolfe e438d53
    New 'loan' and 'company' categories