search

AmpersandJS / ampersand-select-view

Select field for ampersand-form-views
MIT License
11 stars 26 forks source link

Bugfix/handle empty options #45

Closed cdaringe closed 9 years ago

cdaringe commented 9 years ago

Closes #44

wraithgar commented 9 years ago

+1

fyockm commented 9 years ago

Aside from my personal preference toward ternary, looks good. +1 from me. Would love to see this released soon!

cdaringe commented 9 years ago

@fyockm @wraithgar is the combo of your two +1s OK to close, or are we sticking to core_team +1 && module-at-hand-maintainer +1s only? i searched around and couldnt find it. i know it was discussed it somewhere

wraithgar commented 9 years ago

@cdaringe +1 from core OR leads http://ampersandjs.com/learn/bug-triage-process

In my humble opinion :shipit:

As far as ternary goes, in the age of minification we try to lean on the side of longhand for if statements.

cdaringe commented 9 years ago

@wraithgar, drats, i knew it was somewhere! just to be clear, i interpret your remark above as stating that (gar + 1) && (fyockm + 1) === sufficient to merge. however, i interpret your link above to exclude fyockm's +1, as he isnt this module's lead. set me straight!

wraithgar commented 9 years ago

I was following the spirit vs the letter of the law on that given that @fyockm is a lead in another ampersand module. Probably should let someone else besides me weigh in. @AmpersandJS/core-team can we get a +1 and/or some discussion about the triage process now that we have many more leads?

HenrikJoreteg commented 9 years ago

I see three competent people with vested interest in the project all wanting these changes, I've never been one for strict rules. I'd say ship it, especially in these cases where there's little to no opposition.

cdaringe commented 9 years ago

and that's why I like workin' with you guys :+1: