LP2Kanban

Merge lp://qastaging/~gmb/lp2kanban/dont-rely-on-external_system_url into lp://qastaging/~launchpad/lp2kanban/trunk

  1. dont-rely-on-external_system_url
  2. Merge into trunk
Proposed by Graham Binns
Status: Merged
Approved by: Graham Binns
Approved revision: 95
Merged at revision: 65
Proposed branch: lp://qastaging/~gmb/lp2kanban/dont-rely-on-external_system_url
Merge into: lp://qastaging/~launchpad/lp2kanban/trunk
Prerequisite: lp://qastaging/~gmb/lp2kanban/cards2workitems
Diff against target: 587 lines (+272/-60)
6 files modified
src/lp2kanban/bugs2cards.py (+5/-5)
src/lp2kanban/cards2workitems.py (+15/-4)
src/lp2kanban/kanban.py (+66/-5)
src/lp2kanban/tests/common.py (+38/-6)
src/lp2kanban/tests/test_bugs2cards.py (+100/-15)
src/lp2kanban/tests/test_cards2workitems.py (+48/-25)
To merge this branch: bzr merge lp://qastaging/~gmb/lp2kanban/dont-rely-on-external_system_url
Reviewer Review Type Date Requested Status
Benji York (community) code Approve
Review via email: mp+143700@code.qastaging.launchpad.net

Commit message

Stop relying on external_system_url for blueprint links since they can be overwritten merge proposals, etc.

Description of the change

(copied from parent branch):

So, because I'm a bit touched in the brainpan, I decided to have a crack at storing metadata in the description field. Revisions 81-90 of this branch represent that work.

I've had to make some modifications to the test stubs to make everything work nicely. The new functionality is as follows:

 - Metadata can be stored in the card description as key=value pairs. Anything that's not key=value is ignored.
 - Metadata can be retrieved via card.description_annotations, set with card._setDescriptionAnnotations() (if you want to overwrite everything).
 - card.description_annotations is a ReadOnlyRecord, so you can use attributes to access annotations, but you can't write to those attributes (this protects devs from data loss if they foolishly try to write to such an attribute).
 - Individual annotation C,U and D come in the form of .writeAnnotation() and .removeAnnotation().
 - The cards2workitems code still allows you to use the external_system_url field to put a blueprint in, but if it encounters this it will also store the blueprint url in the description annotations for safekeeping (users shouldn't have to edit annotations directly, but they can if they so wish).

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

This branch is eminently landable as-is. I had a couple of
questions/thoughts while reading it.

I'm not quite clear on why the annotations aren't just a read/write
record or dictionary-like object. The "this protects devs from data
loss" bit confuses me.

I'm torn between the current implementation for storing the values and a
JSON-based format.

The current approach is nice to look at and edit. The benefit of JSON
would be that it is richer (arrays, objects, embedded newlines, etc.).

If we decided to go with JSON my first suggestion would be to divide the
description into "text before the first {", "text after the last }", and
"text between the first { and last } (including the braces themselves".
That approach would make it easy to preserve the text around the JSON
and using Python's "json" module would make the (un)serializing easy.

The tests look really good.

review: Approve (code)
lp://qastaging/~gmb/lp2kanban/dont-rely-on-external_system_url updated
93. By Graham Binns

Added some logging.

Revision history for this message
Graham Binns (gmb) wrote :

On 17 Jan 2013, at 16:56, Benji York <email address hidden> wrote:

> Review: Approve code
>
> This branch is eminently landable as-is. I had a couple of
> questions/thoughts while reading it.

Thoughts are always good :). I'm replying to this on a plane. This has nothing to do with the branch, but it made me smile as I wrote it.

> I'm not quite clear on why the annotations aren't just a read/write
> record or dictionary-like object. The "this protects devs from data
> loss" bit confuses me.

I had problems dealing with things like

  card.description_annotations.foo = "bar"

This might be because I tried to implement writing a __setitem__[] that would write back to the description (i.e. call writeAnnotation()). The trouble was that I ended up in infinite recursion land.

Now that I write that, though, I think I see a simpler solution to the problem.

… Yep, I was overcomplicating things. Revision 94 provides this.

> I'm torn between the current implementation for storing the values and a
> JSON-based format.
>
> The current approach is nice to look at and edit. The benefit of JSON
> would be that it is richer (arrays, objects, embedded newlines, etc.).
>
> If we decided to go with JSON my first suggestion would be to divide the
> description into "text before the first {", "text after the last }", and
> "text between the first { and last } (including the braces themselves".
> That approach would make it easy to preserve the text around the JSON
> and using Python's "json" module would make the (un)serializing easy.

I think think using JSON would be ideal, actually. I'll start a separate branch for that.

lp://qastaging/~gmb/lp2kanban/dont-rely-on-external_system_url updated
94. By Graham Binns

Implemented directly-writeable description annotations.

95. By Graham Binns

Removed unused tests.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches