Add status receiver for GitHub #635

adiroiban · 2013-02-13T14:43:54Z

Problem description

GitHub provides a simple API for publishing build status for a revision from a repo.

It would be nice is Buildbot could work with GitHub to publish status for builders.

Here is my try.

Please let me know if this is on the right track and what needs to be changed.

This tried to do the same think as: #516

Use cases for the plugin

Simple projects with a builder using a single repo on internal network and mirrored to GitHub. Define properties on builder and use Interpolation to resolve GitHub repo owner and name. Revision is defined by 'revision' Property.
Project with multiple builders for the same repo (multiple repos) but only a single builder (selected builders) will send status to GitHub. Ex same revision tested on Linux and Windows. Define properties on builder and return empty string to skip status sending.
Simple projects with a builder using a single repo which is hosted in GitHub. Repository owner and name is obtained from Git repo. Revision is defined by 'revision' Property.
Project with a builder using multiple repos but which should send status only to a single github repo. Define properties on builder and use gitHubRepo Interpolation.

Changes requested during review

Rename to github.py and force absolute imports
Use txgithub for non blocking calls
Move documentation in main documentation and not in the source code
Add an user defined configuration named 'gitHubRepo' to GitHubStatus. This allows using an Interpolate to specify the name of GitHub repo where to send status.
Find or add a little tool which can generate a token based on username, password, application keys. This can be an external project and just link to the code.
Add exception handling
Add tests

Changes description

This depend on a patch made to txgitub to support github changes. Hope the patch will be accepted soon.

It uses a token for authentication and I have documented how one can obtain a token.

No testes are present. If everything is ok I will to write the tests.

It depends on an external txgithub package but I tried to avoid reinventing the wheel.

How to try and test the changes

Get txgithub version from here tomprince/txgithub#1:

Check documentation for instruction.

Watch a pull request in GitHub.

tomprince · 2013-02-16T03:03:07Z

There is a related pull request #516.

Also, using a blocking api from the main thread is not good.

adiroiban · 2013-02-16T08:35:34Z

I am aware of #516. To me, it looks like the original author is no longer interested into that branch.

Are you still interested into having this code into main buildbot, or do you prefer it as an external extension?

I don't like that implementation since it does not uses a token and it has its own implementation of github api.

Are you ok with using pygithub for working with GitHub? Would it be ok to use deferToThread?

Thanks!

djmitche · 2013-02-16T20:59:14Z

master/buildbot/status/github_status.py

@@ -0,0 +1,221 @@
+# This file is part of Buildbot.  Buildbot is free software: you can


The filename is redundant. It should just be github.py. I know we have a bunch of other redundantly-named status modules, but that doesn't make it right..

I can not name it github.py since "github" is already used by PyGitHub and then I can not do "from github import Stuff" since it will import from itself.

I would be happy to rename it to github.py. Is there a workaround for this ?

You can use absolute imports - see http://docs.python.org/release/2.5/whatsnew/pep-328.html

Many thanks for this advice. I will switch to absolute import right away! Happy to see this fixed in latest python version!

Well, it was fixed years ago in Python-2.5, but we supported Python-2.4 until relatively recently.

djmitche · 2013-02-16T21:08:41Z

Using pygithub with deferToThread is an acceptable solution. Certainly using the latest GitHub API is a big plus, as is using OAuth2 and a library that will keep up with API updates. PyGithub seems to add substantial functionality.

I would like to get something of this sort merged into to Buildbot - I think this is an important capability lacking in Buildbot right now.

Two things I'd like to see:

Use the sourcestamps in the build, via codebases, to determine which repository to comment on, rather than hard-coding that in the builder. Many users will have builders that build code from multiple repositories.
Flexibility for later expansion to also comment on pull requests, using build properties to indicate the originating pull request.

adiroiban · 2013-02-16T22:42:28Z

Hi. Thanks for the review.

I will update it to deferToThread and add documentation and tests. The blocking code was only as a simple showcase.

I will use sourcestamps to send status. I agree with your usecase.

I am using an internal git mirror for buildot, so Buildbot repository name/URL is not the same as GitHub repository URL.

By default, I can send status updated to all repos from the sourcestamps, but allow overwriting this behavior.

Can you please explain how do you want the "comment on pull requests" feature to work?

Cheers,
Adi

djmitche · 2013-02-17T05:07:32Z

I probably spoke too soon about pull requests. I think the code already in this pull request could be easily adapted to that end once we have a scheduler that pays attention to pull requests.

adiroiban · 2013-02-17T10:44:18Z

Thanks for the review.

Please check "Changes requested during review" from the pull description and let me know if something is wrong or is missing.

I will leave the discussion about "comment on pull requests" for another review. First we need to add a scheduler which listen for GitHub notifications or polls GitHub status for a repo. This is on my TODO list.

adiroiban · 2013-02-17T10:56:45Z

I have also updated the list of "Use cases for the plugin" . I am not sure if we should add "alwaysUseGithubRepo" options, or just use "resolveGitHubRepo" to solve all other custom logic.

Please check the list and let me know if something is wrong or is missing

djmitche · 2013-02-17T14:13:27Z

The changes requested looks good. Per discussion in the commit, I think it's best to omit resolveGitHubRepo and only use alwaysUseGithubRepo, with the possibility of rendering that value. In fact, let's just call it githubRepo. Supply a sensible default for this that achieves the simple use-cases.

(Minor note -- we should be consistent in capitalizing GitHub with its capital 'H', as the company itself does - so gitHubRepo is probably most correct).

adiroiban · 2013-02-17T15:08:21Z

Thanks. I will use GitHub and gitHub names.

gitHubRepo should take care of defining on which GitHub repo to send the status.

What property should be used by default for sending the SHA.

It is ok to use the "revision" property?
Or should I use the revision property from the first SourceStamp repository?

How should this plugin handle the case where multiple repos are used for a builder?
Do we want to send status for all repos or only for some of the repos from SourceStamps.
I have no experience with what are the requirement for this usecase.

djmitche · 2013-02-17T17:14:05Z

The default should probably use the revision of the default codebase (the empty string). So that means Interpolate("%(src::revision)s"). Users with multiple codebases will need to add their own config.

tomprince · 2013-02-23T22:14:52Z

I'd guess that watching for buildsets, rather than builds is what often would be desired.

Regarding token genertaion, txgithub has that.

adiroiban · 2013-02-24T21:38:24Z

Hi @tomprince ,

How a Buildbot Status plugin, can access the Buildsets.

This is what I found about buildset, but I don't know how I can handle them from the plugin:
http://buildbot.net/buildbot/docs/0.8.7p1/manual/concepts.html#buildsets

…name.

adiroiban · 2013-03-19T10:26:01Z

Hi @djmitche

I tried to push this forward by submitting another round of review request.

This was delayed since I am waiting for an update in txgithub.

Please re-read pull description as I have updated it to reflect latest changes.

I have no experience with projects built from multiple repositories so I don't know how we should support them.

I think that we should release the plugin with support for single repositories, and when we receive feedback I volunteer to implement the requested changes.

It would be nice if for simple projects, the status could just look for a Git Step and get the repo_owner and repo_name from that URL.

I don't know how this can be done.

This code is untested but I will write the tests once the general lines are ok.

adiroiban · 2013-03-28T09:06:54Z

Hi @djmitche . Did you had time to check latest changed in documentation and implementation?

This branch still waits for an official release of txgithub tomprince/txgithub#2.

The GitHub communication is in one way and is isolated in a method. I think that we can mock the request and continue with this work.

Before putting more work into this I would like to know if the current direction is right.

Thanks!

djmitche · 2013-03-29T03:04:51Z

master/buildbot/status/github.py

+  SUCCESS: 'success',
+  FAILURE: 'failure',
+  EXCEPTION: 'error',
+}


There are more than these three states, so there should be a fallback when indexing this dictionary.

djmitche · 2013-03-29T03:13:38Z

Huh, i could have sworn I replied earlier, but that was during PyCon. Sorry. Anyway, I've had a look at the code, and it looks fine.

As for supporting multiple projects, we will need to have support for that up-front. Here's why: we've merged a number of incomplete implementations with promises to finish them later, and in many cases that hasn't worked out -- probably for perfectly valid reasons such as changing jobs or no longer needing the functionality. This is unfair both to users, who expect fully-implemented features, and to maintainers, who must eventually make the decision to rip out the feature or spend time implementing the unfinished portions.

jaredgrubb · 2013-04-12T02:52:56Z

master/buildbot/test/unit/test_status_github.py

+        self.assertEqual('1 hours, 1 minutes', result)
+
+        result = self.status._timeDeltaToHumanReadable(1, 60 * 60 + 62)
+        self.assertEqual('1 hours, 1 minutes, 1 seconds', result)


As another example you dont have here, _timeDeltaToHumanReadable would return
"1 day, 1 second"
as a result ... the purist in me thinks that
"1 day, 0 hours, 0 minutes, 1 second"
is actually better.

But, just a personal subjective opinion.

I added the missing test.

Maybe this is cultural difference. I wanted to create a human readable format, and in normal communications from person to person, I say that the build took 1 day and 1 second and never "1 day, 0 hours, 0 minute and 1second".

This code is is not used in this branch so we can get rid of it.

As discussed with Dusting I will implement the start/end description interpolation in another branch.

Let me know if you want me to remove this code.

No, I have no objection. It was more a passing thought. Just thought I'd point it out in case you didnt intend it, or in case someone else has a strong opinion.

adiroiban · 2013-04-14T10:00:04Z

Thanks @djmitche and @jaredgrubb for the review.

I have updated the test and documentation.

Please review the changes and let me know if anything else need to be changes.

Cheers

jaredgrubb · 2013-04-14T17:10:49Z

master/docs/manual/cfg-statustargets.rst

+    repoName = Interpolate("%(prop:github_repo_name)s"
+    sha = Interpolate("%(src::revision)s")
+    startDescription = Interpolate('Build started.')
+    endDescription = Interpolate('Build done.')


I'd remove the Interpolate from these 'description' ones too.

I have removed the startDescription and endDescription and will expand them when I will implement the special interpolation for them.

Yes, you removed them from the code. But you should remove them from these docs too.

djmitche · 2013-04-14T17:23:45Z

These are all good points, and it seems like we're zeroing in on "finished." As far as I'm concerned, we can merge when the remaining details are taken care of.

jaredgrubb · 2013-04-14T17:40:59Z

@adiroiban: Great job on this by the way! This is a great addition to buildbot.

adiroiban · 2013-04-14T21:00:20Z

Thanks @jaredgrubb for the feedback and the github_repo_name questions is very good :)

I tried to expand the documentation with an example of how to define the github_repo_name on a builder.

Please let me know if this make sense or otherwise I am happy to update the documentation.

Cheers

DamnWidget · 2013-05-12T23:12:03Z

Hi.

Any progress with this feature? Sould be really cool to have it merged in the master branch.

adiroiban · 2013-05-13T08:01:34Z

AFAIK the code is ok, but it would help to get some test from other persons.

@DamnWidget if you have time, it would help if you could test the code and validate it based on your requirements.

Thanks!

kyle-johnson · 2013-05-23T23:34:18Z

The status receiver seems to cause exceptions when I enable it. It does send status updates to GitHub, but it halts builds immediately when enabled:

2013-05-23 16:24:24-0700 [-] <Build NLG-Core new>.startBuild
2013-05-23 16:24:24-0700 [github] fetching 'https://api.github.com/repos/foo/NLG-Core/statuses/bfa41cf49a2ddb79a44056c2e2491a0b4fb0106b'
2013-05-23 16:24:24-0700 [-] <Build NLG-Core new>.buildException
2013-05-23 16:24:24-0700 [-] Unhandled Error
        Traceback (most recent call last):
          File "/home/buildbot/bleeding_bot/sandbox/lib/python2.6/site-packages/Twisted-13.0.0-py2.6-linux-x86_64.egg/twisted/internet/defer.py", line 293, in addCallbacks
            self._runCallbacks()
          File "/home/buildbot/bleeding_bot/sandbox/lib/python2.6/site-packages/Twisted-13.0.0-py2.6-linux-x86_64.egg/twisted/internet/defer.py", line 575, in _runCallbacks
            current.result = callback(current.result, *args, **kw)
          File "/home/buildbot/bleeding_bot/chevah/master/buildbot/process/build.py", line 297, in _startBuild_2
            self.startNextStep()
          File "/home/buildbot/bleeding_bot/chevah/master/buildbot/process/build.py", line 395, in startNextStep
            d = defer.maybeDeferred(s.startStep, self.remote)
        --- <exception caught here> ---
          File "/home/buildbot/bleeding_bot/sandbox/lib/python2.6/site-packages/Twisted-13.0.0-py2.6-linux-x86_64.egg/twisted/internet/defer.py", line 137, in maybeDeferred
            result = f(*args, **kw)
          File "/home/buildbot/bleeding_bot/chevah/master/buildbot/process/buildstep.py", line 551, in startStep
            self.step_status.stepStarted()
          File "/home/buildbot/bleeding_bot/chevah/master/buildbot/status/buildstep.py", line 226, in stepStarted
            self.build.stepStarted(self)
          File "/home/buildbot/bleeding_bot/chevah/master/buildbot/status/build.py", line 314, in stepStarted
            receiver = w.stepStarted(self, step)
        exceptions.AttributeError: Deferred instance has no attribute 'stepStarted'

2013-05-23 16:24:24-0700 [-]  <Build NLG-Core new>: build finished
2013-05-23 16:24:24-0700 [github] fetching 'https://api.github.com/repos/foo/NLG-Core/statuses/bfa41cf49a2ddb79a44056c2e2491a0b4fb0106b'
2013-05-23 16:24:24-0700 [-] releaseLocks(<BuildSlave 'Slave 1'>): []
2013-05-23 16:24:25-0700 [_GithubPageGetter (TLSMemoryBIOProtocol),client] Status "pending" sent for foo/NLG-Core at bfa41cf49a2ddb79a44056c2e2491a0b4fb0106b.
2013-05-23 16:24:25-0700 [_GithubPageGetter (TLSMemoryBIOProtocol),client] Status "error" sent for foo/NLG-Core at bfa41cf49a2ddb79a44056c2e2491a0b4fb0106b.

Tried on master with this PR pulled in and tried just the branch this PR is from. Same results both times.

Happy to help track this down, but I'm a bit clueless on how to trace Twisted defers. :)

jaredgrubb · 2013-05-24T01:13:43Z

So 'w' here is a deferred. Looks like a deferred got added into the self.watchers list via the subscribe method.

kyle-johnson · 2013-05-24T05:58:25Z

Thank you, that was enough to get me hunting and figure out what's going on. :)

buildStarted() shouldn't return a deferred that doesn't include a stepStarted method. Of course, there's still a bunch of deferred logic which must occur when a build is started, so the simple solution (borrowing from the TinderboxMailNotifier) is to put the deferred logic in a separate method which is called from buildStarted().

Here's roughly what I'm using:

    def buildStarted(self, builderName, build):
        self._startingStatus(builderName, build)

    @defer.inlineCallbacks
    def _startingStatus(self, builderName, build):
        status = yield self._getGitHubRepoProperties(build)
        if not status:
            defer.returnValue(None)

        (startTime, endTime) = build.getTimes()

        description = yield build.render(self._startDescription)

        status.update({
            'state': 'pending',
            'description': description,
            'builderName': builderName,
            'startDateTime': datetime.datetime.fromtimestamp(
                startTime).isoformat(' '),
            'endDateTime': 'In progress',
            'duration': 'In progress',
            })
        result = yield self._sendGitHubStatus(status)
        defer.returnValue(result)

adiroiban · 2013-05-24T08:16:53Z

Thanks @kyle-johnson for testing. Sorry for the error. I am now updating the code and tests and will push the changes.

adiroiban · 2013-05-24T08:34:31Z

Hi. I have fixed the code and updated the tests. Please check that everything is ok.
Many thanks!

djmitche · 2013-05-26T01:30:15Z

Let's merge once that fix is in. @jaredgrubb, if I get pulled away from reviews again, please feel free to merge directly.

adiroiban · 2013-05-27T07:54:12Z

Hi. Thanks for the review. Is there a better way to fix this? other than logging the deferred error?

I have extended the tests to check for the missing scenarios and used self.successResultOf(d) insted of d.addCallback(result.append).

For logging the error, i used log.err . Please let me know if I should use log.msg.

Thanks!

djmitche · 2013-05-27T14:28:52Z

This looks great! No, at some point things "go wrong" and the best you can do is log them.

The tests for logging are very thorough. I'm a little worried about using the private _loggedErrors, but we'll see how it works on the various Twisted versions.

djmitche · 2013-05-27T14:56:05Z

Two minor problems, then. First, the tests require txgithub, and fail without. Since we haven't added that requirement to setup.py (and I'd rather not do so), the tests should really skip if that module is not available, rather than fali.

Second, I get two errors:

===============================================================================
[ERROR]
Traceback (most recent call last):
  File "/home/dustin/code/buildbot/t/buildbot/master/buildbot/test/unit/test_status_github.py", line 307, in test_sendFinishStatus_ok
    'duration': '2 seconds',
  File "/home/dustin/code/buildbot/t/buildbot/sandbox/lib/python2.7/site-packages/mock.py", line 863, in assert_called_with
    raise AssertionError(msg)
exceptions.AssertionError: Expected call: mock({'description': 'Build done.', 'buildNumber': '1', 'duration': '2 seconds', 'repoOwner': 'repo-owner', 'builderName': 'builder-name', 'endDateTime': '1970-01-01 02:00:03', 'sha': '123', 'state': 'success', 'repoName': 'repo-name', 'startDateTime': '1970-01-01 02:00:01', 'targetURL': 'http://domain.tld'})
Actual call: mock({'repoOwner': 'repo-owner', 'description': 'Build done.', 'builderName': 'builder-name', 'endDateTime': '1969-12-31 19:00:03', 'buildNumber': '1', 'sha': '123', 'state': 'success', 'repoName': 'repo-name', 'startDateTime': '1969-12-31 19:00:01', 'targetURL': 'http://domain.tld', 'duration': '2 seconds'})

buildbot.test.unit.test_status_github.TestGitHubStatus.test_sendFinishStatus_ok
===============================================================================
[ERROR]
Traceback (most recent call last):
  File "/home/dustin/code/buildbot/t/buildbot/master/buildbot/test/unit/test_status_github.py", line 249, in test_sendStartStatus_ok
    'duration': 'In progress',
  File "/home/dustin/code/buildbot/t/buildbot/sandbox/lib/python2.7/site-packages/mock.py", line 863, in assert_called_with
    raise AssertionError(msg)
exceptions.AssertionError: Expected call: mock({'description': 'Build started.', 'buildNumber': '1', 'duration': 'In progress', 'repoOwner': 'repo-owner', 'builderName': 'builder-name', 'endDateTime': 'In progress', 'sha': '123', 'state': 'pending', 'repoName': 'repo-name', 'startDateTime': '1970-01-01 02:00:01', 'targetURL': 'http://domain.tld'})
Actual call: mock({'repoOwner': 'repo-owner', 'description': 'Build started.', 'builderName': 'builder-name', 'endDateTime': 'In progress', 'buildNumber': '1', 'sha': '123', 'state': 'pending', 'repoName': 'repo-name', 'startDateTime': '1969-12-31 19:00:01', 'targetURL': 'http://domain.tld', 'duration': 'In progress'})

buildbot.test.unit.test_status_github.TestGitHubStatus.test_sendStartStatus_ok
-------------------------------------------------------------------------------
Ran 21 tests in 0.030s

FAILED (errors=2, successes=19)

my packages:

DateTime==2.12.6
Jinja2==2.6
MySQL-python==1.2.3
Pygments==1.5
SQLAlchemy==0.7.9
Sphinx==1.1.3
Tempita==0.5.1
Twisted==13.0.0
buildbot==0.8.7p1-584-gb885a23
buildbot-sample-plugin==0.1.0
buildbot-slave==0.8.7p1-584-gb885a23
buildbot-www==0.8.9-pre-479-gcd23d1e
coverage==3.4
decorator==3.4.0
distribute==0.6.30
docutils==0.9.1
epydoc==3.0.1
mock==0.8.0
pg8000==1.08
psycopg2==2.3.2
pyOpenSSL==0.13
pyflakes==0.7.2
python-dateutil==1.5
pytz==2010o
sqlalchemy-migrate==0.7.2
txgithub==0.1.0
unicode-nazi==1.1
unittest2==0.5.1
wokkel==0.7.0
wsgiref==0.1.2
zope.interface==4.0.5

…tart and end datetime.

adiroiban · 2013-05-27T17:07:10Z

I skipped the tests when txgithub could not be imported.

I could not find a setUpClass for twisted.trial.TestCase. This is why I added the skip in setup. It will show 21 skipped tests instead of a single skipped test.

I have updated the date time test and I hope this time they will pass.

Please check the latest changes.

Thanks!

djmitche · 2013-05-27T17:33:57Z

Twisted doesn't do setUpClass, I think because it's based on an older unittest module.

Passes for me -- I'm merging now.

bilderbuchi · 2014-02-21T15:29:10Z

master/buildbot/status/github.py

+        """
+        Convert Buildbot states into GitHub states.
+        """
+        # GitHub defines `success`, `failure` and `error` states.


actually, there's "pending", too, nowadays: http://developer.github.com/v3/repos/statuses/#parameters-1

The 'pending' state is sent in _sendStartStatus . This method is here to convert Buidbot states into GitHub final states.

Why would you want to send 'pending' state to GitHub as the final message for a build?

ah, I see. sorry, got lead astray by the (currently incorrect) formulation of L205. Never mind, then. :-)

Initial GitHub status receiver.

c0ca477

djmitche reviewed Feb 16, 2013
View reviewed changes

adiroiban mentioned this pull request Mar 5, 2013

Update hooks methods. Add repository status methods. tomprince/txgithub#1

Merged

adiroiban mentioned this pull request Mar 19, 2013

Add Github Commit Status status plugin. #516

Closed

adiroiban added 3 commits March 19, 2013 11:17

Switch to txgihub and use interpolation for resolving repo owner and …

2f03606

…name.

Move documentation into user manual.

954caa2

Move documentation into user manual.

37f44f7

djmitche reviewed Mar 29, 2013
View reviewed changes

jaredgrubb reviewed Apr 12, 2013
View reviewed changes

Add release not an dependency on txgithub. Update code after review.

e178ae5

jaredgrubb reviewed Apr 14, 2013
View reviewed changes

Update documentation of repoOwner and repoName usage.

5f267ce

Merge branch 'master' into github_status

cb07649

Fix callbacks for buildStarted and buildFinished.

3811085

Log start and finish errors.

2edcfa6

adiroiban added 2 commits May 27, 2013 20:02

Skip tests when txgithub is not installed. Use dynamic datetime for s…

c50d244

…tart and end datetime.

Refactor to more precise detection of txgithub import error.

44f24d8

djmitche merged commit 44f24d8 into buildbot:master May 27, 2013

bilderbuchi reviewed Feb 21, 2014
View reviewed changes

adiroiban deleted the github_status branch February 16, 2015 11:11

		@@ -0,0 +1,221 @@
		# This file is part of Buildbot. Buildbot is free software: you can

Add status receiver for GitHub #635

Add status receiver for GitHub #635

Conversation

adiroiban commented Feb 13, 2013

Problem description

Use cases for the plugin

Changes requested during review

Changes description

How to try and test the changes

tomprince commented Feb 16, 2013

adiroiban commented Feb 16, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djmitche commented Feb 16, 2013

adiroiban commented Feb 16, 2013

djmitche commented Feb 17, 2013

adiroiban commented Feb 17, 2013

adiroiban commented Feb 17, 2013

djmitche commented Feb 17, 2013

adiroiban commented Feb 17, 2013

djmitche commented Feb 17, 2013

tomprince commented Feb 23, 2013

adiroiban commented Feb 24, 2013

adiroiban commented Mar 19, 2013

adiroiban commented Mar 28, 2013

Choose a reason for hiding this comment

djmitche commented Mar 29, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adiroiban commented Apr 14, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djmitche commented Apr 14, 2013

jaredgrubb commented Apr 14, 2013

adiroiban commented Apr 14, 2013

DamnWidget commented May 12, 2013

adiroiban commented May 13, 2013

kyle-johnson commented May 23, 2013

jaredgrubb commented May 24, 2013

kyle-johnson commented May 24, 2013

adiroiban commented May 24, 2013

adiroiban commented May 24, 2013

djmitche commented May 26, 2013

adiroiban commented May 27, 2013

djmitche commented May 27, 2013

djmitche commented May 27, 2013

adiroiban commented May 27, 2013

djmitche commented May 27, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment