Code review comment for lp:~linaro-infrastructure/launchpad/workitems-schema-changes

On Sun, Feb 5, 2012 at 9:50 PM, Stuart Bishop
<email address hidden> wrote:
> Review: Approve db
>
> Mostly fine.

Good to hear. Thanks.

>
> Do we need a 'name' column on SpecificationWorkItem? If we need to address them individually in the URL space for the web UI or restful API, we might prefer a name rather than expose the internal id.

I don't think we really need it, at least not for the user end of
things. We wouldn't pass around url:s to work items as we do with bugs
for instance, we'd link to the specification instead to reach the
complete set of work items for that specification.

We could add a name column, but it should be automatically generated
so the user would not have to enter a name for each work item. That
would just add overhead to something that's supposed to be really
light weight. Could we start out with just using the internal id for
API calls and maybe add a namn column later if we need it?

>
> Please rename the 'date' column on SpecificationWorkItemChange to 'date_created'. This is consistent, and will avoid headaches from using a keyword as a column name.

Absolutely, will do.

>
> I don't really like the 'day' column name on SpecificationWorkItemStats, but I can't think of a better alternative at the moment.

How about day_harvested or something like that?

>
> We need some indexes:
>
> -- Foreign key, selecting by specification and sorting by date_created.
> CREATE INDEX specificationworkitem__specification__date_created__idx
>    ON SpecificationWorkItem(specification, date_created);
>
> -- Foreign key.
> CREATE INDEX specificationworkitem__milestone__idx
>    ON SpecificationWorkItem(milestone);
>
> -- Foreign key, required for person merge.
> CREATE INDEX specificationworkitem__assignee__idx
>    ON SpecificationWorkItem(assignee) WHERE assignee IS NOT NULL;
>
>
> -- Foreign key, selecting by work_item and ordering by date_created
> CREATE INDEX specificationworkitemchange__work_item__date_created__idx
>    ON SpecificationWorkItemChange(work_item, date_created);
>
> -- Foreign key.
> CREATE INDEX specificationworkitemchange__new_milestone__idx
>    ON SpecificationWorkItemChange(new_milestone)
>        WHERE new_milestone IS NOT NULL;
>
> -- Foreign key, required for person merge.
> CREATE INDEX specificationworkitemchange__new_assignee__idx
>    ON SpecificationWorkItemChange(new_assignee) WHERE new_assignee IS NOT NULL;
>
> -- Foreign key, and selection by date.
> CREATE INDEX specificationworkitemstats_specification__day__idx
>    ON SpecificationWorkItemStats(specification, day);
>
> -- Foreign key, required for person merge.
> CREATE INDEX specificationworkitemstats__assignee__idx
>    ON SpecificationWorkItemStats(assignee) WHERE assignee IS NOT NULL;
>
> -- Foreign key.
> CREATE INDEX specificationworkitemstats__milestone__idx
>    ON SpecificationWorkItemStats(milestone) WHERE milestone IS NOT NULL;

So we'll add that to the patch then.

> --
> https://code.launchpad.net/~linaro-infrastructure/launchpad/workitems-schema-changes/+merge/91295
> Your team Linaro Infrastructure is subscribed to branch lp:~linaro-infrastructure/launchpad/workitems-schema-changes.