At the moment, we stuff the treestatus reason field full of generic tree messages / MOTD type strings.
...to add a new motd/message type field to TreeStatus, so people don't accidentally overwrite the MOTD when closing the tree/adding a reason (as happened last night).
Once the new field has been added, TBPL will need to concatenate reason+motd at:
I guess there's nothing stopping us from moving the MOTD parts/formatting them differently/even maybe not showing them if the tree is CLOSED (to prevent the situations where we end up with 3 million lines of text due to a long closure message).
Created attachment 657203 [details] [diff] [review]
As suggested, if the tree is closed, it will display status+reason, and if open status + reason + motd.
It's also gated on the presence of the motd field in the JSON returned from treestatus. If that field isn't present (or empty) it will display status+reason. In particular, this avoids having to co-ordinate pushing this patch with the treestatus landing - it can be deployed immediately with no visible change in behaviour, and will automatically do the right thing when the treestatus.m.o change is deployed. We can then circle back later and remove the "&& data.message_of_the_day" part of the condition.
Comment on attachment 657203 [details] [diff] [review]
r=me, but I think we should move the:
+ if (data.reason)
+ baseMessage += '. ';
before the cssStatus == "open" conditional, so that the end of sentence punctuation doesn't vary depending on whether the tree is open or closed. This will also make the if-else clearer.
+ } else
Please can we go for braces on both parts, or if we move the |if (data.reason)| per above, then we could have both without. Just not a huge fan of mix and match in the same if-else.
(And sorry for the delay, was away on Friday).
And thank you for doing this :-D
Missed getting marked as depending on bug 790559, since there was bug number in the commit.
Anyway, this is in production now (and has been for a day or so) :-)
(In reply to Ed Morley [:edmorley UTC+1] from comment #6)
> Missed getting marked as depending on bug 790559, since there was bug number
> in the commit.
no bug number, even :-)