[Buildbot-devel] Slave concurrency?

[Jean-Paul Calderone]

On Fri, 22 Jun 2007 13:08:29 -0500, "Dustin J. Mitchell" <dustin at zmanda.com> wrote:
>On Thu, Jun 21, 2007 at 02:08:31PM -0500, Dustin J. Mitchell wrote:
>> Here's my solution:
>
>Sorry to spam the list, but I just posted a series of four patches that
>implement the solution in the quoted message.  They're all fairly simple
>patches, so I'd appreciate it if anyone could take a look and let me
>know what you think.
>
>I can send the patches as attachments if your mail client is unhappy
>with the inline text.

Since you asked... ;)

I quickly skimmed the two patches and two things struck me:

  * there were no new unit tests or changes to existing unit tests.  I can't
    decide which of these two things is more alarming.

  * Changing the name of the isAvailable method will break any third-party
    modules which overrode or called this method.  It would be better to
    leave the method name alone or provide a backward compatibility layer.

Hope this is useful,

Jean-Paul