launchpad-dev team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/13/2010 05:53 AM, Julian Edwards wrote:
> On Tuesday 12 October 2010 15:23:34 Aaron Bentley wrote:
>> On 10/11/2010 05:33 AM, Julian Edwards wrote:
>>> On Friday 08 October 2010 16:49:54 Aaron Bentley wrote:
>>>> On 10/08/2010 11:25 AM, Julian Edwards wrote:
>>>>> Michael went to great lengths to ensure that his work was backwardly
>>>>> compatible and that nobody was forced to do anything, and from my point
>>>>> of view I thought he did an incredible job.
>>>>
>>>> From my perspective, backwards compatibility was not a virtue.  We did
>>>> not want to be left behind.
>>>
>>> Absolutely - it was just there for breathing space so we could land the
>>> work without immediately impacting you guys.
>>
>> I think it would have been more productive to "go all the way" and
>> implement it on the remaining types, rather than spent time and effort
>> on backwards compatibility.  The backwards compatibility work did
>> immediately impact us.  It uglified the code, and introduced bugs and
>> conflicts in the code I was working on.
> 
> This directly contradicts our mantra of landing code as soon as possible.

I think the mantra is "land it as soon as it's an improvement", and
while there was some improvement, there was also the creation of tech
debt, which kinda balances it out.

> I'm interested to see examples of the ugly code, bugs and conflicts though, as 
> they should have been avoided.

Many BuildBase instance methods were converted into staticmethods that
took an instance as a parameter: getUploaderCommand,
getUploadLogContent, handleStatus,  handleStatus, _handleStatus_OK,
_handleStatus_PACKAGEFAIL, _handleStatus_DEPFAIL,
_handleStatus_CHROOTFAIL, _handleStatus_BUILDERFAIL,
_handleStatus_GIVENBACK, storeBuildInfo, queueBuild

For example:
- -    def queueBuild(self, suspended=False):
+    @staticmethod
+    def queueBuild(build, suspended=False):
         """See `IBuildBase`"""
- -        specific_job = self.makeJob()
+        specific_job = build.makeJob()

         # This build queue job is to be created in a suspended state.
         if suspended:
             specific_job.job.suspend()

- -        duration_estimate = self.estimateDuration()
+        duration_estimate = build.estimateDuration()
         queue_entry = BuildQueue(
             estimated_duration=duration_estimate,
- -            job_type=self.build_farm_job_type,
+            job_type=build.build_farm_job_type,
             job=specific_job.job, processor=specific_job.processor,
             virtualized=specific_job.virtualized)
- -        Store.of(self).add(queue_entry)
+        Store.of(build).add(queue_entry)
         return queue_entry

It's been a few months, but I believe there were two problems:
1. I was overriding _handleStatus_OK as an instance method, but it was
now a staticmethod.
2. The base implementation of _handleStatus_OK was always being invoked,
regardless of whether it had been overridden.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAky1uyEACgkQ0F+nu1YWqI3DXACfTb7ZlHQYsmIXN/Q9CAFqt19+
6QwAn3TKXlHM8PUoNZnoBY+8yVVkc+5w
=GN2I
-----END PGP SIGNATURE-----