Tag Archives: plugins

The Joy of Coding (Episode 7): Code review, and a Regression

In this episode, I started with some code review. I was reviewing a patch to make the Findbar (particularly, the Find As You Type feature) e10s-friendly.

With that review out of the way, I had to swap a bunch of information about the plugin crash UI for e10s in my head – and in particular, some non-determinism that we have to handle. I explained that stuff (and hopefully didn’t spend too much time on it).

Then, I showed how far I’d gotten with the plugin crash UI for e10s. I was able to submit a crash report, but I found I wasn’t able to type into the comment text area.

After a while, I noticed that I couldn’t type into the comment text area on Nightly, even without my patch. And then I reproduced it in Aurora. And then in Beta. Luckily, I couldn’t reproduce it in Release – but with Beta transitioning to Release in only a few days, I didn’t have a lot of time to get a bug on file to shine some light on it.

Luckily, our brilliant Steven Michaud was on the case, and has just landed a patch to fix this. Talk about fast work!

Episode Agenda

References:
Bug 1133981 – [e10s] Stop sending unsafe CPOWs after the findbar has been closed in a remote browser

Bug 1110887 – With e10s, plugin crash submit UI is broken – Notes

Bug 1147521 – Cannot type into comment area of plugin crash UI

The Joy of Coding (Episode 6): Plugins!

In this episode, I took the feedback of my audience, and did a bit of code review, but also a little bit of work on a bug. Specifically, I was figuring out the relationship between NPAPI plugins and Gecko Media Plugins, and how to crash the latter type (which is necessary for me in order to work on the crash report submission UI).

A minor goof – for the first few minutes, I forgot to switch my camera to my desktop, so you get prolonged exposure to my mug as I figure out how I’m going to review a patch. I eventually figured it out though. Phew!

Episode Agenda

References:
Bug 1134222 – [e10s] “Save Link As…”/”Bookmark This Link” in remote browser causes unsafe CPOW usage warning

Bug 1110887 – With e10s, plugin crash submit UI is broken – Notes

Python Eggs: Sunny Side Up, and Other Goodies (or How I Learned to Stop Worrying and Start Coding)

Cooking with Eggs

Every now and then, the computer gods smile and give me a freebie.

I’ve been worrying my mind out over a few problems / obstacles for my Review Board extensions GSoC project.  In particular, I’ve been worrying about dealing with extension dependencies, conflicts, and installation.

I racked my brain.  I came up with scenarios.  I drew lots of big scary diagrams on a wipe board.

And then light dawned.

Batteries Come Included

Enter Setuptools and Python Eggs.

All of those things I was worried about having to build and account for?  When using Python Eggs, It’s all built in. Dependencies?  Taken care of. Conflicts?  Don’t worry about it.  Installation?  That’s what Setuptools and Python Eggs were built for!

In fact, it even looks like Setuptools was designed with extensible applications in mind.

Wait, really?  How?

Here’s the setup.py file for the rb-reports extension in the rb-extensions-pack on Github:

from setuptools import setup, find_packages

PACKAGE="RB-Reports"
VERSION="0.1"

setup(
    name=PACKAGE,
    version=VERSION,
    description="""Reports extension for Review Board""",
    author="Christian Hammond",
    packages=["rbreports"],
    entry_points={
        'reviewboard.extensions':
        '%s = rbreports.extension:ReportsExtension' % PACKAGE,
    },
    package_data={
        'rbreports': [
            'htdocs/css/*.css',
            'htdocs/js/*.js',
            'templates/rbreports/*.html',
            'templates/rbreports/*.txt',
        ],
    }
)

Pay particular attention to the “entry_points” parameter.  What this is doing, is registering rbreports.extension:ReportsExtension to the entry point “reviewboard.extensions”.

“Hold up!”, I hear you asking. “What’s an entry point?”

Entry Points

An entry point is a unique identifier associated with an application that can accept extensions.

The unique identifier for Review Board extensions is “reviewboard.extensions”.

This is the first handshake, more or less, between Review Board and any extensions:  in order for Review Board to “see” the extension, the extension must register an entry point at “reviewboard.extensions”.

This blog post shows how extensions can be found and loaded up.

Other Goodies

INSTALLED_APPS and Django

I remember also being worried about how to create tables in Django for extension models.  I thought “holy smokes, I’m going to have to either shoehorn some raw SQL into the extension manager, or maybe even trust the extension developers to write the CREATE TABLE queries themselves!”.

Luckily, there’s a better alternative.

Django knows about its applications through a dictionary called INSTALLED_APPS. When you add a new model to a Django project, you simply add the model app to the INSTALLED_APPS dictionary, and run “manage.py syncdb”.  Django does the magic, bingo-bango, and boom – tables created.

So if a new extension has some tables it needs created, I simply insert the app name of the extension into INSTALLED_APPS when the extension is installed, and call syncdb programmatically.  Tables created:  no sweat.

django-evolution

Creating tables is easy.  But what if an extension gets updated, and the table needs to be modified?  Sounds like we’ve got a mess on our hands.

And don’t expect Django to save you.  When you modify a model in Django, they expect you to into that DB and alter that table by hand:

[syncdb] creates the tables if they don’t yet exist. Note that syncdb does not sync changes in models or deletions of models; if you make a change to a model or delete a model, and you want to update the database, syncdb will not handle that.
From The Django Book – Chapter 5: Models

Thankfully, there’s a mechanism that’s already built into Review Board that makes this trouble go away:  django-evolution.  Django-evolution, when used properly, will automatically detect changes in application models, and alter the database tables accordingly.  This is how Review Board does upgrades.

And to top that off, RB co-founder Christian Hammond just became the django-evolution maintainer.

Wow.  Everything is falling neatly into place.

My GSoC Project: Review Board Extensions

If you didn’t already know, Review Board is an open-source web-based code review tool.  The MarkUs Team has been using Review Board for pre-commit code review for about a year now.  This has given the team a number of advantages:

  1. For a team that usually has a 4 month turnover, this allows us to quickly get new team members up to speed with how to contribute to MarkUs.  We review every change that they propose, and give them tips/guidance on how to make it fit in well with the application.  They learn, and the applications code stays healthy.
  2. We catch defects before they enter the code base.  Simple as that.
  3. We get a good sense of what other people are working on, and what is going on in the code.  Review Board has become a central conversation and learning hub for the developers on the MarkUs team.

So, the long and the short of it:  I like Review Board.  Review Board helps us write better code.  I want to make Review Board better.

So what am I proposing?

How to Avoid A Bloated Software Monster

You can never make some people happy.

No matter how decent your software is, someone will eventually come up to you and say:

Wow!  Your software would be perfect if only it had feature XYZ!  Sadly, because you don’t have feature XYZ, I can’t use it.  Please implement, k thx!

And so you either have to politely say “no”, and lose that user, or say “yes”, and add feature XYZ to the application.  And for users out there who don’t need, or don’t care about feature XYZ, that new feature just becomes a distraction and adds no value.  Make this happen a bunch of times, and you’ve got yourself a bloated mutha for a piece of software.

And we don’t want a bloated piece of software.  But we do want to make our users happy, and provide feature XYZ for them if they want it.

So what’s the solution?  We provide an extension framework (which is also sometimes called a plug-in architecture).

An extension framework allows developers to easily expand a piece of software to do new things.  So, if a user wants feature XYZ, we (or someone else) just creates and make available an extension that implements the feature.  The user installs the extension, activates it, and bam – our user is happy as a clam with their new feature.

And if we make it super-easy to develop them, third-party developers can write new, wonderful, interesting extensions to do things that…well, we wouldn’t have considered in the first place. It’s a new place for innovation.  What’s that old cliché?

If you build it [the plug-in framework], they will come [the third-party developers who write awesome things]

And the developers do come.  Just look at Firefox add-ons or WordPress plugins.  Entire ecosystems of extensions, doing things that the original developers would probably have never dreamed of doing on their own.  Hell, I’ve even written a Firefox add-on. And users love customizing their Firefox / WordPress with those extensions.  It adds value.

So we get wins all over the place:

  • Our user gets their feature
  • The software gets more attractive because it’s flexible and customizable
  • The original software developers get to focus on the core piece of software, and let the third-party developers focus on the fringe features

And this is where I think I can help Review Board.

(Before I go on, if you’re interested, here’s another article on the how and the why of plug-in architectures)

Review Board Extensions

So if you look at the Review Board Wiki, or glance at the mailing lists you see numerous requests from users for new features, for example:

It would be nice if the review board had a “next comment” button that is always available to click, or had a collapse/expand button. This would make it easier to see other people’s comments in cases like this.

…

It will be nice to have post-commit support. Instead of every post-commit review being a separate URL, if we could setup default rules for post-commit reviews to update an existing review providing the diff-between-diff features, it would be very useful.

The Review Board developers could smell the threat of bloated feature-creep from a mile away.  So, in a separate branch, they began working on integrating an extension framework into Review Board.

The extension branch, however, has been gathering dust, while the developers focus on more critical patches and releases.

My GSoC proposal is to finish off a draft of the extension framework, document it, and build a very simple extension for it.  My simple extension will allow me to record basic statistics about Review Board reviewers – for example, how long they spend on a particular review, their inspection rate, etc.

Having been a project lead MarkUs for so long, it’s going to be a good experience to be back on “the bottom” – to be the new developer who doesn’t entirely have a sense of the application code yet.  It’s going to be good to go code spelunking again.  I’ve done some preliminary explorations, and it’s reminding me of my first experiences with MarkUs.  Like a submarine using its sonar, I’m slowly getting a sense of the code terrain.

I’ll let you know what my first few sweeps find.

ReviewBoard Extensions: A State of Affairs

One of my potential research experiments involves augmenting ReviewBoard, and so, I’ve been studying the ReviewBoard code.

I want to give ReviewBoard the ability to record certain statistics – for example, the number of defects in a review request, number of defects found per reviewer, defect density, inspection rate, etc…

It turns out I’m not the only one who wants this to happen.  Check out this thread.

So it turns out that the devs building ReviewBoard are planning on integrating an extension framework.  Theoretically, users could then write their own extensions to customize ReviewBoard as they see fit.  Think WordPress plugins, but for a code review tool.  Neat.

According to the above link:

A lot of this [extension framework] already exists in a private development branch, and it will be one of our primary focuses as soon as 1.0 goes out.

However, I found the following quote from a relatively recent mailing list message:

[with regards to the extensions framework]…We have to finish building it, testing
it, getting other features we have planned into the release, and performing
the release. So, we’re looking at a year or more. But I don’t see how we’d
get the functionality you want short-term.

Hrmph.  My thinking:  maybe I can help them with it, and we can crank this puppy out sooner.

So where is this private branch?  It actually took a little bit of detective work…

Detective Work

I started by looking at the ReviewBoard repo on Github, which I assumed that the extensions branch would fork from.

I was right – check out the fork network.  There’s a branch helpfully labeled “extensions” forked by davidt, one of the ReviewBoard devs.

Two observations:

  1. This fork hasn’t been updated in about a month
  2. The extensions stuff references stuff from Djblets which doesn’t exist in the Djblets master branch

So I perform the same exercise – I glance at the Djblets fork network, and see the handily named “extensions” fork that is in davidt’s git repo.  Nice.

Ok, so those are the forks/branches that I’m going to grab.

A Fluke

So, funny story:  I was able to get this “extension” version of ReviewBoard working on my work machine.  But it was by a complete fluke – this extension version of Djblets is actually missing some pieces.  The only reason it worked for me was because I accidentally ran with the master checkout of Djblets first.  This errored out, and also generated a bunch of compiled Python.  I then switched to the extensions branch, which left the compiled Python behind.  The compiled Python filled all of the missing pieces that are in the extensions branch of Djblets, and so it ran.

For a guy who is pretty new to both Git and Django, I found that problem pretty frustrating to track down.  Especially when I started writing up the checkout instructions… big thanks to Zuzel for her invaluable (and patient) Django guidance.

Anyhow, if you’re interested in the steps to failure, here they are…

Setting Up (for failure)

I’m running Ubuntu Karmic.  I have Git installed.

  1. First of all, this document is invaluable
  2. Check out django_evolution from SVN to some local directory
  3. Put path to django-evolution into PYTHONPATH (export PYTHONPATH=$PYTHONPATH:/…/django-evolution-read-only/)
  4. Get Python Setuptools (sudo apt-get install python-setuptools)
  5. With easy_install, get nose, paramiko, recaptcha-client, Sphinx (sudo easy_install nose paramiko recaptcha-client sphinx)
  6. Clone original master Djblets repo (git clone git://github.com/djblets/djblets.git) and CD into djblets directory
  7. Add davidt’s remote Djblets branch (git remote add davidt git://github.com/davidt/djblets.git)
  8. Fetch davidt’s repo  (git fetch davidt)
  9. Track davidt’s extension branch for djblets (git branch extensions davidt/extensions)
  10. In djblets root dir, type:  sudo python setup.py develop
  11. This should install Django too.  The ReviewBoard install doc says to install from Subversion, but this will do for now.
  12. Clone ReviewBoard git repo (git clone git://github.com/reviewboard/reviewboard.git) and Cd into it
  13. Add davidt’s remote Reviewboard branch (git remote add davidt git://github.com/davidt/reviewboard.git)
  14. Fetch davidt’s repo  (git fetch davidt)
  15. Track davidt’s extensions branch of reviewboard (git branch extensions davidt/extensions)
  16. Switch to extensions branch (git checkout extensions)
  17. So reviewboard/reviewboard/extensions should have stuff in it.
  18. python ./contrib/internal/prepare-dev.py
  19. Manually create the directory reviewboard/reviewboard/htdocs/media/ext.  ReviewBoard expects it and will complain if it isn’t there.
  20. Run ./contrib/internal/devserver.sh
  21. Go to localhost:8080

If you’re like me, you’ll see an error about “no module named urls”.  This is because the extension branch of Djblets is missing urls.py in its djblets/log directory.  As you can see, urls.py exists in the master Djblets branch.

So I’ve accidentally got it running.  But this version of ReviewBoard I’ve created is like Frankenstein’s monster – it’s a sad, stitched together monstrosity that should probably never see the light of day.

I tried to merge the master branch of Djblets and ReviewBoard into the extensions branch, and then realized that I had no idea what I was doing.  The last guy you want doing a merge is a guy who doesn’t know what he’s doing.

So that’s where I stand with it.  Kind of a sad affair, really.  I’ve posted on the ReviewBoard developer mailing list asking for some guidance, but all is quiet so far.

In the meantime, I’ll continue studying the code when I can.  The extensions stuff is actually pretty far along, in my opinion – but what do I know, I’ve never built one.  I’ll also take a peek at some other examples of extension frameworks to see what I can learn about implementation (that’s right – I’m talking about you, WordPress).

I’ll let you know what I find.