Mentored bugs would really benefit from having the bug's mentor in the needinfo dropdown. The convention we use is to specify the mentor in the whiteboard, ie: [mentor=name@email.tld] There is the occasional case where the mentor is specified as an IRC nick, but we can just discourage that practice.

while the review extension provides a bug.mentor field which supports both fully qualified and irc nick entries in the mentor field, it can be slow due to the additional searches required, so i'd like to avoid bringing that onto show_bug. once we have a field for bug mentors (bug 649691), then that no longer becomes a concern.

I realize you would rather wait on the proper Mentor extension that brings it's own tables for storing the mentor values, but this particular request was simple enough to implement using current functionality and will be converted automatically once the new extension is complete. bug.mentors currently provided by extensions/BMO/Extension.pm will move to extension/Mentor/Extension.pm and the return data should be the same. Feel free to hold off on this review if you feel like it would be a drain on performance but I thought I would put it out there anyway. dkl

(In reply to David Lawrence [:dkl] from comment #2) > bug.mentors > currently provided by extensions/BMO/Extension.pm will move to > extension/Mentor/Extension.pm and the return data should be the same. Actually after thinking about this some more, we can just leave it in the Review extension and add the needed changes there. dkl

Comment on attachment 8375734 [details] [diff] [review] 943146_1.patch this _needs_ bug 649691 implemented first, as generating the mentor list right now is expensive. clearing review until that's ready.

Comment on attachment 8442203 [details] [diff] [review] 943146_2.patch Review of attachment 8442203 [details] [diff] [review]: ----------------------------------------------------------------- r=glob ::: extensions/Needinfo/template/en/default/bug/needinfo.html.tmpl @@ +100,5 @@ > } else if (role == 'user') { > identity = '[% user.realname || user.login FILTER html FILTER js %]'; > + [% FOREACH mentor = bug.mentors %] > + } else if (role == '[% mentor.login FILTER js %]') { > + identity = '[% mentor.realname || mentor.login FILTER html FILTER js %] (mentor)'; nit: indent the code within the FOREACH loop

Comment on attachment 8442203 [details] [diff] [review] 943146_2.patch Review of attachment 8442203 [details] [diff] [review]: ----------------------------------------------------------------- i was thinking about this some more, and i think for the common case where there's only one mentor the <select> should say just "mentor". if there's more than one mentor then it makes sense to use the current implementation and show each mentor's email address.

Comment on attachment 8442880 [details] [diff] [review] 943146_3.patch Review of attachment 8442880 [details] [diff] [review]: ----------------------------------------------------------------- thanks, that looks much better. there's some redundancy that needs to be removed and then we should be good to go :) ::: extensions/Needinfo/template/en/default/bug/needinfo.html.tmpl @@ +100,5 @@ > } else if (role == 'user') { > identity = '[% user.realname || user.login FILTER html FILTER js %]'; > + [% FOREACH mentor = bug.mentors %] > + } else if (role == '[% mentor.login FILTER js %]') { > + identity = '[% mentor.realname || mentor.login FILTER html FILTER js %] (mentor)'; there's no need to append "mentor" if there's only one mentor, because that information is already provided by the selected option. @@ +141,5 @@ > <option value="qa_contact">qa contact</option> > [% END %] > <option value="user">myself</option> > + [% FOREACH mentor = bug.mentors %] > + <option title="mentor" value="[% mentor.login FILTER html %]"> this tooltip is redundant when there's only one mentor.

Comment on attachment 8443504 [details] [diff] [review] 943146_4.patch Review of attachment 8443504 [details] [diff] [review]: ----------------------------------------------------------------- r=glob

To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git f16dc32..4e8e717 master -> master

I hereby VERIFY that the needinfo dropdown includes "mentor" on BMO bugs having a nonempty "mentor" field in the header. Examples: bug 137014, bug 96972