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

Enable narratives to source multiple builds - Add Ons #1167

Conversation

salvatore-fxpig
Copy link
Contributor

Description of proposed changes

Just adding some magic sauce to #1164

Dunno if you've pulled in my fork, this may make it easier on your end (or not!)

@salvatore-fxpig salvatore-fxpig changed the title 1050 mult narratives more Enable narratives to source multiple builds - Add Ons Jun 12, 2020
@salvatore-fxpig salvatore-fxpig force-pushed the 1050-mult-narratives-more branch 2 times, most recently from 1ce32ea to 6c322d9 Compare June 12, 2020 16:18
@jameshadfield
Copy link
Member

This looks really good to me (I haven't actually faked a slow connection to test the case where we change to a narrative slide who's cached json is a pending promise, but it's within an async function so it should work). I'm also a fan of avoiding lodash functions (or to any similar library) when we can accomplish the same functionality using reduce, map etc (or avoid it altogether as done here 😉 ). @eharkins I'll leave it to you to merge into your PR as you see fit.

@eharkins eharkins temporarily deployed to auspice-1050-mult-narra-7uf2fo June 15, 2020 22:45 Inactive
@salvatore-fxpig salvatore-fxpig force-pushed the 1050-mult-narratives-more branch from f5962ec to 591134e Compare June 16, 2020 16:00
@jameshadfield jameshadfield temporarily deployed to auspice-1050-mult-narra-7uf2fo June 16, 2020 16:01 Inactive
@eharkins
Copy link
Contributor

Merged into #1164, thank you @salvatore-fxpig!

@eharkins eharkins closed this Jun 17, 2020
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