| * erikhatcher leaves | 00:48 | |
| * ndushay leaves | 04:43 | |
| * erikhatcher joins | 05:26 | |
| * bess joins | 09:17 | |
| * erikhatcher leaves | 09:47 | |
| * erikhatcher joins | 09:52 | |
| * bess leaves | 10:27 | |
| * ndushay joins | 13:27 | |
| * ndushay leaves | 13:30 | |
| * ndushay joins | 13:51 | |
| * jamieorc joins | 14:32 | |
| <jrochkind> | ndushay: (or anyone) else. Poll about my patch. Right now, my patch DOES require a change in your blacklight_config.rb. It's not backwards compat with blacklight_config.rb. I could make it so, at the cost of more lines of code. Any opinions on if that's necessary? | 14:43 |
| * Naomi joins | 15:01 | |
| * bess joins | 15:02 | |
| * ndushay leaves | 15:18 | |
| <jrochkind> | in case anyone is actually here now, I'll try one more time, and then figure nobody cares. :) | 15:20 |
| Right now, my patch DOES require a change in your blacklight_config.rb. It's not backwards compat with blacklight_config.rb. I could make it so, at the cost of more lines of code. Any opinions on if that's necessary? | ||
| <bess> | hey, jrochkind, sorry I'm on a conf call so not really here :) | 15:24 |
| <jrochkind> | no prob. I tried asking on the listserv too. I'm just trying to be concientious, but if nobody cares, then nobody cares, and I'll leave it how it is, which was my gut. | |
| * mmitchell joins | 15:51 | |
| ndushay: u 'round? | 15:52 | |
| <ndushay> | on same conf call as bess. | |
| <bess> | hi, ndushay :) | |
| <ndushay> | i think we need to poll more broadly | 15:53 |
| jrochkind: maybe send an email JUST about the config file changes to list | ||
| b/c it's for anyone currently using blacklight | ||
| having to twiddle | ||
| but should be easy twiddle. | ||
| (just a coupla edits to blacklight config file, yes?) | ||
| <erikhatcher> | jrochkind: seems like your patch, best i recall of it, could easily handle backwards compatibility here, right? might as well do that | 15:54 |
| <mmitchell> | ndushay: ok | 15:55 |
| jrochkind: nice patch jonathan. tests, comments and all that ;) | 15:56 | |
| <jrochkind> | mmitchell: thanks. I tried to make it as nice as I could, cause I'm still trying to butter you all up. :) Please note that while I was at it I even added tests for functionality that was there before, untouched by my patch, but didn't yet have tests. :) | 15:57 |
| Although I guess the reason I added those tests, was cause I re-wrote teh method in question. But the method in question didn't have tests before. | ||
| <mmitchell> | jrochkind: I noticed that, thank you | |
| <jrochkind> | erikhatcher: Right now my patch does not do backwards compat. But yes, I could make it do so, jsut by sniffing to see if you have an Array as a value, instead of the new Hash. Question was, do we have enough of an install base to make it worth the code cruft. Which is what I wasn't able to get anyone to have an opinion on. But if you think yes, I can add in the backwards compat sniff. | 15:58 |
| erikhatcher: I mean, you can DO the same thing youd id before in my patch. Right now you just need to rewrite your config to use Hashes instead of Arrays to do it. | 15:59 | |
| It's _functionally_ backwards compat. It just isn't backwards compat wtih the actual blacklight_config.rb file itself, blacklight_config.rb would need to be modified. Unless I add a backwards compat sniff. | ||
| <erikhatcher> | jrochkind: sounds like a simple if statement (if not a ternary operator) | |
| in these cases where it's simple enough, i'd say just go the extra mile (err, inch) | 16:00 | |
| <jrochkind> | erikhatcher: it's an if that just converts the Array into the new form Hash meaning the same thing, if the Array is present. I trust your judgement. Okay, I'll add it. | |
| mmitchell: so based on erikhatcher opinion, only one expressed, I'll go add backwards compatibility for old blacklight_config.rb in. So don't apply/merge the patch until I do that, but that'll just be 5 lines or so. | 16:01 | |
| <erikhatcher> | i'm saying this kinda blindly.... just thinking that it's one less thing someone would need to do to upgrade that might be a pain | |
| <mmitchell> | ok sounds good to me | |
| <jrochkind> | yeah, I could go either way, sounds fine to me. | 16:02 |
| like you say, easy enough to do. Just trying to keep un-needed cruft out of the code. But if someoen else thinks it sounds convenient enough to justify, easy enough to do. | 16:03 | |
| <mmitchell> | hmm, great release notes could do the job too | 16:05 |
| <erikhatcher> | but sounds like it might be useful with a Hash or an Array, no? so maybe it's good to be flexible? | 16:06 |
| <jrochkind> | sure, why not. | 16:07 |
| <mmitchell> | true | |
| <jrochkind> | old style: ['label', 'qt_param'] turns into: {:display_label => "label", :qt => 'qt_param} | |
| Personally I prefer the latter for clarity even if that is all you're doing. And personally I prefer not having a billion ways to do it just for the sake of flexibility. But seeing as how the first one is legacy in addition to being slightly more terse, why not. | 16:08 | |
| A two-element array is all it'll recognize though -- the legacy two element array. You want anything more than that, hash. | ||
| * ndushay leaves | 16:14 | |
| <jrochkind> | heh, as usual, the test is more of a pain than the code. | 16:15 |
| * bess leaves | 16:40 | |
| * mmitchell leaves | 17:06 | |
| * bess joins | 17:19 | |
| * bess leaves | 17:26 | |
| * bess joins | 17:27 | |
| * bess leaves | 17:28 | |
| * ndushay joins | 17:52 | |
| * Naomi joins | 17:53 | |
| * ndushay leaves | 17:55 | |
| * Naomi joins | 17:57 | |
| * jamieorc leaves | ||
| * bess joins | 18:04 | |
| * bess leaves | 18:11 | |
| * ndushay leaves | 18:15 | |
| * cbeer_ joins | 18:20 | |
| * bess joins | 18:49 | |
| * bess leaves | 18:54 | |
| * bess joins | 19:13 | |
| * bess leaves | 19:44 | |
| * ndushay leaves | 20:46 | |
| * ndushay joins | 21:24 | |
| * ndushay leaves | 22:07 | |
Generated by Sualtam