Code review comment for lp:~bac/launchpad/bug-828572

Revision history for this message
Francis J. Lacoste (flacoste) wrote :

Hi Brad,

Thanks for working on this stakeholders' escalated bug.

You didn't list anybody in the Pre-implementation section, did you discuss with anyone if this change would be appropriate?

I fear that solution is inadequate for a few reasons:

* The original bug mention this would also affect expiration, this solution doesn't address that. (Not necessarily a big problem, you could easily file another bug about the expiration aspect.)

* If for some reason the user is available and respond within one minute, the bug will still be considered without response. That might be a corner case, but it's still possible and would result in a bug being filed.

I'd suggest approaching this from another angle: don't update date_last_message for changes that shouldn't be considered for expiration or don't really consider a response. In a way, date_last_message should only be updated for "new messages", not for status, importance, tags, or any other non-message related changes. That should solve both problems.

It would leave all existing bugs that had their date_last_message wrongly set show up in the searches. We might consider a data migration for that (update the date_last_message field based on the date of the... last message for all incomplete bug. We should do that in a garbo task.)

review: Needs Fixing

« Back to merge proposal