Bug 89215 - master.cfg cleanup: Pass CheckOutSource instance instead of class to BuildStep.addStep
Summary: master.cfg cleanup: Pass CheckOutSource instance instead of class to BuildSte...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Csaba Osztrogonác
URL:
Keywords:
Depends on:
Blocks: 89001
  Show dependency treegraph
 
Reported: 2012-06-15 08:11 PDT by Csaba Osztrogonác
Modified: 2012-06-26 08:03 PDT (History)
4 users (show)

See Also:


Attachments
Patch (2.29 KB, patch)
2012-06-15 08:14 PDT, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2012-06-15 08:11:41 PDT
because it is deprecated and will be dropped in buildbot 0.8.7
Comment 1 Csaba Osztrogonác 2012-06-15 08:14:12 PDT
Created attachment 147822 [details]
Patch

Tested on a local test buildmaster (same as build.webkit.org)
Comment 2 Tony Chang 2012-06-20 15:41:15 PDT
Comment on attachment 147822 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147822&action=review

> Tools/ChangeLog:6
> +
> +        Reviewed by NOBODY (OOPS!).

Nit: Can you include the 'why' (what you wrote in comment #0) in the changelog?

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:117
> +    def __init__(self, **kwargs):

Nit: Do we need **kwargs here? I only see one caller for CheckOutSource and it doesn't pass any flags.
Comment 3 Csaba Osztrogonác 2012-06-26 07:57:24 PDT
Comment on attachment 147822 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147822&action=review

>> Tools/ChangeLog:6
>> +        Reviewed by NOBODY (OOPS!).
> 
> Nit: Can you include the 'why' (what you wrote in comment #0) in the changelog?

Sure, will do.

>> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:117
>> +    def __init__(self, **kwargs):
> 
> Nit: Do we need **kwargs here? I only see one caller for CheckOutSource and it doesn't pass any flags.

Yes, we need. Buildbot will instantiate CheckOutSource too,
we should pass through kwargs to the parent class always.

"Keep a **kwargs argument on the end of your options, and pass that up to the parent class's constructor."
http://buildbot.net/buildbot/docs/0.8.6p1/manual/customization.html#writing-buildstep-constructors
Comment 4 Csaba Osztrogonác 2012-06-26 08:01:40 PDT
Comment on attachment 147822 [details]
Patch

Landed in http://trac.webkit.org/changeset/121263 with additional comment.