"Date requested" in reviewer table is confusing for proposals created WIP

[Julian Edwards]

See https://code.launchpad.net/~julian-edwards/launchpad/timeouts-feature-bug-681426/+merge/41881

Julian started with a WIP proposal and then changed it later, but the Date Requested timestamp is the same as the creation timestamp.

This is because the timestamp reflects the time the review request was created, regardless of whether the proposal "needs review" at that time.

[Aaron Bentley (abentley)]

The date requested is not the same as the creation timestamp of the merge proposal. For example https://code.launchpad.net/~abentley/launchpad/detect-xen-2/+merge/39294 two entirely different timestamps.

The date requested field reflects when an individual reviewer was requested to perform the review (it is the creation timestamp of the review request, not of the merge proposal). It is not meant to indicate when the proposal became ready for review.

[Julian Edwards (julian-edwards)]

Something is wrong still, the dates should not be the same. I guess the default reviewer was added before the review was Needs Review.

[Aaron Bentley (abentley)]

That's right, a review request is added for the default reviewer when you create the merge proposal.

[Aaron Bentley (abentley)]

The bug you describe is not accurate, so we cannot fix it. The data being displayed is accurate.

You say the dates should not be the same, but in order for the dates to differ, we would have to have a bug that prevented the default reviewer from being added when the merge proposal was created.

The values don't mean what you clearly think they should mean. We could try to describe what they mean better, e.g. "Date requested" => "Date selected".

We can remove it, if having a "Date selected" column doesn't make sense.

We can display the date that the merge proposal entered the "Needs review" state. That's not a per-reviewer field, so it wouldn't appear in the review table. If we did that, would we update it each time the proposal entered that state, or only the first time?

[Julian Edwards (julian-edwards)]

On Thursday 25 November 2010 19:26:10 you wrote:
> The bug you describe is not accurate, so we cannot fix it. The data
> being displayed is accurate.
>
> You say the dates should not be the same, but in order for the dates to
> differ, we would have to have a bug that prevented the default reviewer
> from being added when the merge proposal was created.

I think that is the bug then, based on what you're saying. I am simply not
requesting any review from anyone while my MP is "work in progress" so adding
the default reviewer at that stage seems wrong.

> The values don't mean what you clearly think they should mean. We could
> try to describe what they mean better, e.g. "Date requested" => "Date
> selected".
>
> We can remove it, if having a "Date selected" column doesn't make sense.

I was confused by this UI so I think something needs to change. It depends on
what you want to communicate to the user.

For my part, I'd like to see when a MP was moved to "needs review" and I don't
mind how it's done.

> We can display the date that the merge proposal entered the "Needs
> review" state. That's not a per-reviewer field, so it wouldn't appear
> in the review table. If we did that, would we update it each time the
> proposal entered that state, or only the first time?

I think it makes more sense to update each time it enters that state.

Cheers.