Code review comment for lp:~thumper/launchpad/edit-commit-msg-link

Revision history for this message
Māris Fogels (mars) wrote :

Hi Tim,

As discussed on IRC, the calls to commit_message_listener() actually flash the merge link, not the original trigger or message. I couldn't read that in the code, but it makes sense. Perhaps there is some way to make that visual feature a bit clearer.

Using e.halt() is the correct behaviour if you have handled the result. In the case of clicking on a link, you usually want to prevent others from handling a click, so e.halt() is correct.

Please try to clarify the reason for flashing code a bit, and use e.halt(). With those, you will be good to land: r=mars.

Maris

review: Approve (js)

« Back to merge proposal