Jun. 29th, 2010

karzilla: a green fist above the word SMASH! (Default)

Bug counts: resolved 198, assigned 11, needs-review 8



I spent some time today helping [personal profile] cesy with bug 2512, attempting to reproduce the icon browser from the quick reply form on the standard reply page.

My main issue with the initial patch was that it needed better grouping of similar things. That is to say, if the code looks like this:

do thing1 if can_do_it;
do thing2 if can_do_it;
do thing3 if can_do_it;


It's easier to read and maintain if we reformat it like so:

if (can_do_it) {
    do thing1;
    do thing2;
    do thing3;
}


I also said "OMG do we really need all those js files?" [personal profile] fu assured me that we do. Sigh.

I briefly went down a wrong path when I thought we only needed to load the button from ReplyPage.pm, but I later realized we needed to account for sitescheme comment pages as well, which use talkpost.bml and talkpost_do.bml. And we need to load the js files on each of those three different pages that call this function. Double sigh.

There was one last bug in the final submission that [personal profile] fu fixed, which is that we assumed $remote was assigned. It was, but lower down in the file than where we were checking for it.

This will definitely be a target for later refactoring, very similar to what I did for the quote button in bug 2693. Sometimes I have to remind myself not to jump on it right away. Get it working first; refactor it later. This one is more annoying than the quote button, though, because the quote button wasn't loading js libraries...
Page generated Feb. 14th, 2026 07:00 pm
Powered by Dreamwidth Studios