Add support for dot properties in addTest - for #1088 #1089

Merged
merged 1 commit into from Oct 31, 2013

Projects

None yet

3 participants

@SlexAxton
Member

I considered putting something in the options for sub tests, but this was pretty simple and everyone gets it I think. Didn't run tests, so lets wait on that.

/cc @darcyclarke

@SlexAxton
Member

Lol, broke everything. I'll debug for real. I THOUGHT I STILL HAD IT OK?!

@darcyclarke
Contributor

👍 😜

@ryanseddon
Member

You're fired, we don't accept failure on this project.

@SlexAxton
Member

I changed the game and only allow one level of sub tests, which seemed totally reasonable to me.

@SlexAxton
Member

But as a disgraced member of the team, I look towards my elders for guidance.

@SlexAxton
Member

Yea. Seems good. @ryanseddon wanna code-read and merge if you like it?

@ryanseddon ryanseddon commented on an outdated diff Oct 31, 2013
src/addTest.js
@@ -46,6 +46,18 @@ define(['ModernizrProto', 'Modernizr', 'hasOwnProp', 'setClasses'], function( Mo
delete this._l[test];
};
+ function modPath( path ) {
+ var split = path.split('.');
+ var val = Modernizr[split[0]];
+ for (var i = 1; i < split.length; i++) {
@ryanseddon
ryanseddon Oct 31, 2013 Member

If we're only going two levels don't think the loop is necessary anymore

@SlexAxton
Member

pushed a fix.

@ryanseddon
Member

Sweet can you squash please

@SlexAxton SlexAxton Add support for dot properties in addTest - for #1088
Try again with easier/reasonable constraints, and also add feature to async version as well

Dont be silly about how deep to check for path info
4d192fa
@SlexAxton
Member

I SKWASHED OK

@SlexAxton
Member

Gonna merge for darcy and great justice.

@SlexAxton SlexAxton merged commit 66b7034 into Modernizr:master Oct 31, 2013

1 check passed

default The Travis CI build passed
Details
@stucox stucox referenced this pull request Mar 2, 2014
Closed

v3.0 release notes #805

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