The last message in the task log (ex: Updated task status to "Successful") is always send with the drush_log type "info" instead of the result of the operation. It would probably be better if the type would be error for this line if the task is failed.

CommentFileSizeAuthor
#2 give_the_last_message-2568643-2.patch1.15 KBMartijnBraam
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

MartijnBraam created an issue. See original summary.

MartijnBraam’s picture

MartijnBraam CreditAttribution: MartijnBraam commented
Assigned: MartijnBraam » Unassigned
Status: Active » Needs review
FileSize
give_the_last_message-2568643-2.patch1.15 KB

Please review the patch attached.

helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions commented

Looks good, although maybe the default should just be 'info'

  • helmo committed 660be94 on 7.x-3.x authored by MartijnBraam 
    Issue #2568643 by MartijnBraam: Give the last message in the task log...
  • helmo committed 6d78f21 on 7.x-3.x
    Issue #2568643 by MartijnBraam: Set default message type to info
  • helmo committed 6fb6491 on 7.x-3.x
    Issue #2568643 by helmo: code style cleanup
helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions commented
Status: Needs review » Fixed

Commited.
I also updated the default to info and satisfied the coder module.

A few remarks from the coder module:

 31 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
    |         | question marks
 32 | ERROR   | Expected "switch (...) {\n"; found "switch(...){\n"
 35 | ERROR   | Case breaking statements must be followed by a single blank
    |         | line
 38 | ERROR   | Case breaking statements must be followed by a single blank
    |         | line
 41 | ERROR   | Case breaking statements must be followed by a single blank
    |         | line

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

gboudrias’s picture

gboudrias CreditAttribution: gboudrias at Praxis Labs Coop commented
Issue tags: +Aegir 3.2
ergonlogic’s picture

ergonlogic
he/him
Primary language English
Location Montréal, Québec 🇨🇦
CreditAttribution: ergonlogic commented
Status: Closed (fixed) » Active

While I get the reasoning here, I don't know that this is actually a good idea. FWIW, this was how I'd initially implemented this message. The problem is that we're flagging a non-warning or non-error entry as such. I think this is actually misleading.

We have jump links that get you to the first relevant warning or error. I don't know what this would provide.

Sorry for commenting after the fact.

helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions commented

I think it's nice that it matches the overall result of a task.

But I agree with @ergonlogic that the line itself is not an error. And it could also be confusing if someone were to gather statistics on the number of errors.

So I don't object to reverting this.

  • helmo committed dff09d8 on 7.x-3.x 
    Issue #2568643: Revert: Give the last message in the task log the...
helmo’s picture

helmo CreditAttribution: helmo at Initfour websolutions commented
Status: Active » Fixed

I've reverted this ... sorry @MartijnBraam

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.