722580 - Move 'ReviewBoard' and 'Rietveld' BugUrl sub-classes to a new 'MoreBugUrl' extension

Status: RESOLVED FIXED

(Bugzilla :: Extensions, enhancement)

Description

[Tiago Mello [:timello]]

Attachment: v1

Add a new hook and allow more BugUrl sub-classes to be added via extensions, otherwise we will end-up infinitely adding more trackers to the core.

Comment 1

[Frédéric Buclin]

Comment on attachment 592932 [details] [diff] [review]
v1

>=== added file 'extensions/MoreBugUrl/Extension.pm'

>+use constant MORE_SUB_CLASSES => qw(

Maybe add a comment above this constant that this is the single place in this file which should be modified to add new bug trackers. People may think that they must also add code into install_update_db(), which is not the case.


>+sub install_update_db {

Add a comment saying that this code is only for the two bug trackers which were originally put into Bugzilla/BugUrl, and that there is no need to alter this code for new bug trackers, to avoid confusion.


These comments can be added on checkin. r=LpSolit

Comment 2

[Frédéric Buclin]

Hum, I suppose we should add the 'disabled' file into the extension directory to disable it by default, but in that case I wonder how the code would behave if the DB was still containing references to removed Perl modules.

Comment 3

[Frédéric Buclin]

(In reply to Frédéric Buclin from comment #1)
> >+use constant MORE_SUB_CLASSES => qw(
> 
> Maybe add a comment above this constant

Scratch that. The other comment above install_update_db() is enough. No need to add this comment here, especially because those who want to add a new bug tracker should do it in their own extension, not here.


> >+sub install_update_db {
> 
> Add a comment saying that this code is only for the two bug trackers which
> were originally put into Bugzilla/BugUrl, and that there is no need to alter
> this code for new bug trackers

Here too, scratch the end of the sentence about altering the code. Just mention that install_update_db() is here because these two extensions were originally under Bugzilla/BugUrl/.


(In reply to Frédéric Buclin from comment #2)
> Hum, I suppose we should add the 'disabled' file into the extension
> directory to disable it by default

Thinking about this a bit more, we didn't release any development snapshot yet with these two bug trackers, so please add the 'disabled' file into this directory to disable this extension by default, as we do with all other extensions.

Comment 4

[Tiago Mello [:timello]]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/                                                                                                                                    
modified Bugzilla/BugUrl.pm
modified Bugzilla/Hook.pm
missing Bugzilla/BugUrl/ReviewBoard.pm
modified Bugzilla/BugUrl/ReviewBoard.pm
missing Bugzilla/BugUrl/Rietveld.pm
modified Bugzilla/BugUrl/Rietveld.pm
added extensions/MoreBugUrl
added extensions/MoreBugUrl/Config.pm
added extensions/MoreBugUrl/Extension.pm
added extensions/MoreBugUrl/disabled
added extensions/MoreBugUrl/lib
added extensions/MoreBugUrl/template
added extensions/MoreBugUrl/lib/ReviewBoard.pm
added extensions/MoreBugUrl/lib/Rietveld.pm
added extensions/MoreBugUrl/template/en
added extensions/MoreBugUrl/template/en/default
added extensions/MoreBugUrl/template/en/default/hook
added extensions/MoreBugUrl/template/en/default/hook/global
added extensions/MoreBugUrl/template/en/default/hook/global/user-error-bug_url_invalid_tracker.html.tmpl
modified template/en/default/global/user-error.html.tmpl
Committed revision 8117.

Comment 6

[Frédéric Buclin]

Added to relnotes for 4.4.