This function obviously belongs in Bugzilla::Group. However, generally, instead of using this function we should be creating Group objects. It takes about as much time to create a Group object as it does to call this function.
Also, the "GroupExists" function is used only once, and is the *exact same function* as GroupNameToId. So we just use group_name_to_id instead.
Status: NEW → ASSIGNED
Summary: Move GroupNameToId into Bugzilla/Group.pm → Move GroupNameToId into Bugzilla/Group.pm and eliminate GroupExists
Created attachment 213023 [details] [diff] [review] v1 Once again, a very simple patch. :-)
Created attachment 213024 [details] [diff] [review] v2 I improved the POD slightly.
Comment on attachment 213024 [details] [diff] [review] v2 >Index: Bugzilla/Group.pm >=================================================================== >+=item C<group_name_to_id($name)> Nit: You could have also added this to the synopsis section in the POD. I wouldn't mind if the Bugzilla::get_all_groups would be corrected at the same time (should be Bugzilla::Group::get_all_groups). If you want, both can be fixed on checkin. >+ Description: Converts a group name to an id. >+ In general, instead of using this function, you should >+ create a Group object and get its name. This function >+ does not offer any real performance advantage. Makes me think this sub shouldn't exist but the three places where this is used be converted to Group objects.
Attachment #213024 - Flags: review?(wicked) → review+
Okay, did the checkin fix for the POD. Checking in editgroups.cgi; /cvsroot/mozilla/webtools/bugzilla/editgroups.cgi,v <-- editgroups.cgi new revision: 1.66; previous revision: 1.65 done Checking in editproducts.cgi; /cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v <-- editproducts.cgi new revision: 1.114; previous revision: 1.113 done Checking in globals.pl; /cvsroot/mozilla/webtools/bugzilla/globals.pl,v <-- globals.pl new revision: 1.353; previous revision: 1.352 done Checking in Bugzilla/Group.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Group.pm,v <-- Group.pm new revision: 1.11; previous revision: 1.10 done
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Created attachment 213646 [details] [diff] [review] v3 (checked-in version) Okay, here's the version that I checked-in.
Attachment #213024 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.