Skip to content

Conversation

@je-lopez
Copy link

@je-lopez je-lopez commented Sep 13, 2017

Fixes #1070

Overview

Whenever a member's phase has been changed in echo, the phase property of the HubSpot contact for that member should be updated as well in a worker

Notes

Also extracts functions from index allowing only importing/exporting

Copy link
Contributor

@jaredatron jaredatron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. Where are the tests?

@je-lopez
Copy link
Author

je-lopez commented Sep 13, 2017

Test added, and created files for the functions that were in index

Copy link
Contributor

@jaredatron jaredatron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assuming @prattsj doesn't spot anything this looks good to me

contactProfileByEmail: email => `/contacts/v1/contact/email/${email}/profile`,
contactProfileByVID: vid => `/contacts/v1/contact/vid/${vid}/profile`,
}
import {default as getContactByEmail} from './getContactByEmail'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a linter that prefers:

import {default as getContactByEmail} from './getContactByEmail'

over:

import getContactByEmail from './getContactByEmail'

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we do. I think it's xo

heyheyjp
heyheyjp previously approved these changes Sep 14, 2017
@heyheyjp heyheyjp dismissed their stale review September 14, 2017 06:06

Accident.

@heyheyjp
Copy link
Collaborator

Please dismiss my previous review - was an accident. Intended for another PR reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants