add() allowed the inclusion of duplicate extensions #229

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants

rspindel commented Apr 9, 2013

add() did not correctly look for duplicate extensions being added, need to use underscores where() (or findWhere() ) to match objects in a list 

@rspindel rspindel Update aura.extensions.js
add() did not correctly look for duplicate extensions being added, need to use underscores where() (or findWhere() ) to match objects in a list 
45bf4b0
Owner

sbellity commented Apr 9, 2013

Thanks @rspindel

Can you provide some tests where the problem show up ?

rspindel commented Apr 9, 2013

I created another mocha test and it seems to work either way, so my
underscore library may have been dated.

On Tue, Apr 9, 2013 at 5:04 AM, Stephane Bellity
notifications@github.comwrote:

Thanks @rspindel https://github.com/rspindel

Can you provide some tests where the problem show up ?


Reply to this email directly or view it on GitHubhttps://github.com/aurajs/aura/pull/229#issuecomment-16101678
.

R'phael Spindel
704-SPI-NDEL (774-6335)
347-410-WEBS (9327)
http://www.linkedin.com/in/rspindel
http://www.twitter.com/webspin http://www.twitter.com/#!/webspin
SMS: text "webspin" to 50500

Owner

addyosmani commented Apr 9, 2013

Could we double check rspindel/aura@6c2a18c? Seems to be making Travis a little unhappy :)

build failed due to PhantonJS timeout:

Running "mocha:all" (mocha) task
Testing index.html
Warning: PhantomJS timed out, possibly due to a missing Mocha run() call.� Use --force to continue.

I see this happens sometimes...

Owner

addyosmani commented Apr 20, 2013

I created another mocha test and it seems to work either way, so my underscore library may have been dated.

Reading this again, could you confirm if you discovered that there isn't actually an issue with add() or that the tests and PR are good to land? I need to test this again.

Owner

addyosmani commented May 4, 2013

ping. Would still love to land this if we can determine what the underscore issue was.

Owner

addyosmani commented May 19, 2013

From what I've understood re-reading through the discussion log this issue is not a problem using recent versions of underscore without the need to switch to where. If we still run into this as an issue we can re-open the pr.

addyosmani closed this May 19, 2013

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