[WIP] ES2016 locales #64
Conversation
|
@hhsnopek sent you a message on slack regarding this PR - could you take a look at it when you have a minute? |
zspecza
force-pushed
the
es2016-locales
branch
from
d5439f9 to
1387327
Compare
zspecza
force-pushed
the
es2016-locales
branch
from
1387327 to
877d935
Compare
|
Might have to temporarily disable asynchronous tests until Roots has better ways of disabling analytics for test helpers - this means running tests serially, which will take slightly longer, but only temporary, and is just to get CI tests to pass. Otherwise, this is almost complete and just needs updated comments. The conditional logic could be improved a little but it's not important - and there could be a few more tests, but we can add these in the future. @kylemac @hhsnopek care to review anything before I merge this into #55? |
|
@declandewet get those tests 🎾 before we review |
|
@kylemac debugging as we speak. weirdly enough the tests pass locally and have nothing to do with locales so this is somewhat confusing but on it in any case |
zspecza
force-pushed
the
es2016-locales
branch
from
8235430 to
e93577e
Compare
|
@hhsnopek - I've merged this in but could you give it a review anyway (when you have time)? I'd like to hear your thoughts. Any changes can be submitted via PR to |
|
...and we'll address any comments in a new PR |
| @@ -119,11 +193,11 @@ function convert_types_to_array (types) { | |||
| */ | |||
| async function get_all_content (types) { | |||
| types = await Promise.all(types) | |||
| return types.map(async type => { | |||
| let content = await fetch_content(type) | |||
| for (const type of types) { | |||
There was a problem hiding this comment.
Why do you use const instead of let?
There was a problem hiding this comment.
Usually, to denote that type won't be changed but rather a new value will be inferred from type. I'm glad you asked this, however - since further down, I actually do change type by setting type.content. I will refactor this to use let eventually.
|
besides my one question, mainly for my knowledge - I'd like to think there's a better way to accomplish what I did with the locales, tho I'm unsure what to improve without diving into the code myself. 👍 from me 😀 |
Ported from the CoffeeScript version in #39