Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(api): prefer async/await in device/session routes #2301

Merged
merged 2 commits into from
Aug 22, 2019

Conversation

philbooth
Copy link
Contributor

Related to #2286. Contrary to the instructions there though, I've opened this directly against master.

Using one long-lived branch seems more prone to merge conflicts than independently merging short-lived branches as soon as they're ready. And there's nothing that precludes these asyc/await changes from landing bit-by-it.

Note this PR also contains a bonus fix where the path for proxyquire was wrong in the redis tests. Not sure where that came from or how long it's been broken, but I needed it to fix the build.

@mozilla/fxa-devs r?

@philbooth philbooth added this to the Train 145: FxA milestone Aug 22, 2019
@philbooth philbooth requested a review from a team August 22, 2019 08:34
@philbooth philbooth self-assigned this Aug 22, 2019
Copy link
Member

@jaredhirsch jaredhirsch left a comment

Choose a reason for hiding this comment

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

LGTM. Good point about just landing these directly into master; I'll get the main conversion branch in today

@@ -32,7 +32,7 @@ describe('lib/redis:', () => {
};
fxaShared = sinon.spy(() => redis);
wrapper = proxyquire(`${LIB_DIR}/redis`, {
'../../../fxa-shared/redis': fxaShared,
'../../fxa-shared/redis': fxaShared,
Copy link
Member

Choose a reason for hiding this comment

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

oh, I was wondering why this test kept failing locally!

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.

None yet

2 participants