[Buildbot-devel] Adding comments to builds

[Ben Hearsum]

Brian Warner wrote:
> I'm assuming the comments are
> about the build as a whole and not a particular step, so BuildStatus seems
> more appropriate than BuildStepStatus.
> 

That's right, they are for an entire Build.

> We should change the Waterfall display to add a link to the per-Build box
> (the BuildBox class in buildbot.status.html) if the build has a "comments"
> property. We'd need to add a new child link type (something like
> BUILDERNAME/builds/NUMBER/comments) to be the target of this link, and then
> add some code to StatusResourceBuild.getChild that looked like:
> 
>   if path == "comments":
>       return static.Data("text/plain", self.build.getProperty("comments"))
> 
> (at least for a read-only form).
> 

This part has already been taken care of. Tinderbox has some fancy-schmancy
CSS/Javascript to do this. Clicking on any of the 'L' links at
http://tinderbox.mozilla.org/showbuilds.cgi?tree=MozillaTest will give you an
example of this. I think it's a bad idea to require Javascript though, I will
be putting that functionality on the BUILDER/builds/NUMBER page as well.

I've made the following changes to html.py so far:
* Support for Javascript
* A StatusResourceComments, a page similar to the following:
http://tinderbox.mozilla.org/addnote.cgi?log=MozillaTest/1160393280.14046.gz
* Changed StatusResourceBuilder to support returning a StatusResourceComments
* Modified the 'Build NN' link to pop-up a a window like the Tinderbox 'L' link.

> Adding another .control attribute for comments feels overkill to me, so I'm
> inclined to say that IBuildStatus is "mostly read-only" but add setProperty()
> to it.
> 
> From there on out, it's all a question of UI. Probably a little "C" link on
> the yellow Build box (perhaps "c" if no comments have been made yet), that
> takes you to a page with a text form that's been pre-filled with the previous
> comments. 

I'm not sure if I understand correctly, is the intention of pre-filling the
form so that the previous comments are not lost? If that's the case perhaps it
would be better to getProperty("comments") and append the new one to that.

> It might be reasonable to add a Waterfall argument that sets the
> mode: no comments, read-only comments, and read-write comments. This way you
> could have an internal Waterfall page (which allowed Force Build and writing
> comments) and an external one (which didn't allow Force Build and gave
> read-only access to comments). People who didn't want that extra "c" link
> cluttering their pages could use the read-only mode, in which the link was
> only added if there are comments to be read, and then just never add any.

A new Waterfall argument seems appropriate. As I envision it now there will be
some sort of icon beside the 'Build NN' link when there is comments. The 'add
comment' page will be accessible through the pop-up dialog or
BUILDER/builds/NN page.

- Ben