[Buildbot-devel] Adding comments to builds

[Brian Warner]

Ben Hearsum <ben.hearsum at senecac.on.ca> writes:

> For the past few days I have been working on adding something to the
> Waterfall display to allow comments to be added to a build.

This would be pretty cool. I'm trying to think about a good way to work it
into the architecture, though.

The "right place" for these comments to live is in the BuildStatus object.
They want to be in a *Status object somewhere so they can get saved to disk
and become available to other status plugins.. I'm assuming the comments are
about the build as a whole and not a particular step, so BuildStatus seems
more appropriate than BuildStepStatus.

We could add them as "Build Properties", probably one named "comments". We
could also add a new API for them, IBuildStatus.(get|set)Comments, but
somehow I think build properties are sufficient.

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).

Now.. how to get the comments in there in the first place? The biggest
architectural obstacle is that all of the I*Status interfaces are supposed to
be read-only. Should we relax this and allow html.StatusResourceBuild to do a
self.build.setProperty("comments", newcomments) ? Or should we make it obtain
a reference that allows read-write access first? For doing things like
starting and stopping builds, we currently do the latter: the
StatusResourceBuilder has an attribute named .control that (if present)
allows it to force builds. If it has this reference, it can force builds, if
not, it is limited to just looking at status.

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. 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.

I've added SF#1573390 to track this one.

cheers,
 -Brian