Karzilla, Destroyer of Bugs

Bug counts: resolved 367, assigned 10, needs-review 7

I managed to work on 7 "from-suggestions" bugs for the September hackathon. All were effort-minor: 2159, 2821, 2825, 2836, 2838, 3880, 3886.

I also submitted a walkthrough for 2159: http://dw-dev-training.dreamwidth.org/27698.html

Bug counts: resolved 199, assigned 11, needs-review 6

I hadn't been planning on doing any dev work today, but I saw bug 2749 come in and I couldn't resist. I had committed the patch that added AO3 to the list, and I had just been working on bug 733 which touched the same code in DW::External::Site, so I pounced on it.

My first thought was to explicitly disallow any site that didn't have a servicetype of "lj" since I knew we only crossposted to LJ-based sites. That's the same approach I used when dealing with detecting journaltype in 733. I knew I was making an assumption about protocols, and it nagged at me, but it produced the desired result.

However, alierak helped me figure out the better way to do it, which was to ask XPostProtocol for its protocol mappings. That would let me more authoritatively say "these are the sites that can crosspost." And when allen comes along and adds support for a different protocol, that list can update itself without any code changes.

So now in addition to the get_sites method that lists all the defined sites, we also have get_xpost_sites that only lists the sites that can crosspost. Yay!

As a bonus, I saw another optimization that could be made from XPostProtocol and copied it to Site - using LJ::ModuleLoader->autouse_subclasses instead of copying out the list of external site subclasses in the file. Again, one less thing to maintain manually.

Since this is a pretty big change to the header of the same code file I edited in 733, the patches aren't compatible. I pulled my 733 patch from the queue and will regenerate it once my new code from 2749 is approved.

Bug counts: resolved 191, assigned 8, needs-review 7

Today's random cleanup project:

Most people want to print out dates in ISO format, but there's no standard way in the code to do this, so we see code that looks like what just went through changelog for bug 1561:

  my ( $sec, $min, $hour, $mday, $mon, $year, $wday, $yday, $isdst ) = gmtime( $u->timecreate );
  my $journalcreated = sprintf( "%04d-%02d-%02d", $year+1900, $mon+1, $mday );

If there's one lesson I want you to take away from my first post in this journal, it is that code duplication makes me twitch. In this case, it's unlikely that we're ever going to need to update that code (barring some major apocalyptic event rewriting our dates to count from a year other than 1900), but it seems like a good idea to make this a function, just so that people have to write less code every time they want to do this.

I will call my new function LJ::mysql_date - because it is a direct analog to LJ::mysql_time, which already exists in ljtimeutil.pl and does the same thing, except including the time instead of just the date string. I think iso_date and iso_time would be better names, but I'm not prepared to rock the boat that much today.

So now the above code can be made much simpler:

  my $journalcreated = LJ::mysql_date( $u->timecreate, 1 );

The '1' indicates we are using gmtime instead of localtime, as mysql_time also does.

Having done that, another simplification presents itself. Every ID being calculated in this code starts with the exact same form "tag:$LJ::DOMAIN,$journalcreated:$u->{userid}" which is calculated from user data and the site constant $LJ::DOMAIN. That makes it a good candidate for a user method, instead of constructing the string from scratch everywhere it is needed. Furthermore, if we ever need to change the format again (as this bug just did), we can update it in one place.

I will call it $u->atomid and define it as a subroutine in LJ/User.pm, which is the large file where most of the user method declarations live. I will add it to Section 26: Syndication-Related Functions, since it is related to generating the user's atom feed. There's only one other function there at the moment!

kaisa did a great job in the original patch of changing the affected lines from the old format to the new format. I'm simply looking one step further, to say "this should have been packaged more neatly in the first place."

My bug for this work is 2738 and you can view the full patch there.