sanitycheck hooks

RESOLVED FIXED in Bugzilla 3.6

Status

()

Bugzilla
Administration
--
enhancement
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: bbaetz, Assigned: bbaetz)

Tracking

Bugzilla 3.6
Bug Flags:
approval +

Details

(Whiteboard: [es-ita])

Attachments

(1 attachment, 3 obsolete attachments)

8.98 KB, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Assignee)

Description

9 years ago
Created attachment 382091 [details] [diff] [review]
Patch

Some extensions may want to sanitycheck their schemas/etc. We should allow that via hooks.
Attachment #382091 - Flags: review?(mkanat)
(Assignee)

Updated

9 years ago
Whiteboard: [es-ita]

Comment 1

9 years ago
Comment on attachment 382091 [details] [diff] [review]
Patch

>--- sanitycheck.cgi	31 Mar 2009 19:16:13 -0000	1.144
>+Bugzilla::Hook::process("sanitycheck-repair", { status => \&Status });

  Hmm, doesn't it seem like we should be putting that subroutine into some library, instead?

>--- Bugzilla/Hook.pm	18 Dec 2008 17:18:25 -0000	1.24
>+This hook allows for extra sanity checks to be added.

  Add "to F<sanitycheck.cgi>" at the end of that sentence.

>+This hook allows for extra sanity check repairs to be made.

  And there. You might want to describe a bit more what "sanity check repairs" means.

>+++ extensions/example/code/sanitycheck-checks.pl	5 Jun 2009 07:16:52 -0000

  This looks like you included the ITA file, which is too much and too complex for an example.

>+++ extensions/example/code/sanitycheck-repair.pl	5 Jun 2009 07:16:52 -0000

  Same here.

>+  [% ELSE %]
>+    [% message = Hook.process("statuses") %]

  Template hooks don't return anything, as far as I know...does that actually work?
Attachment #382091 - Flags: review?(mkanat) → review-
(Assignee)

Comment 2

9 years ago
(In reply to comment #1)
> (From update of attachment 382091 [details] [diff] [review])
> >--- sanitycheck.cgi	31 Mar 2009 19:16:13 -0000	1.144
> >+Bugzilla::Hook::process("sanitycheck-repair", { status => \&Status });
> 
>   Hmm, doesn't it seem like we should be putting that subroutine into some
> library, instead?

Its just a wrapper arround a custom message template. Not really worth it.

> >+++ extensions/example/code/sanitycheck-checks.pl	5 Jun 2009 07:16:52 -0000
> 
>   This looks like you included the ITA file, which is too much and too complex
> for an example.

Oops. I have an example; looks like I copied the wrong files over.

> >+  [% ELSE %]
> >+    [% message = Hook.process("statuses") %]
> 
>   Template hooks don't return anything, as far as I know...does that actually
> work?

Yes, it works. its the same as assiging the results of a BLOCK or any other subtemplate
(Assignee)

Comment 3

9 years ago
Created attachment 384369 [details] [diff] [review]
v2

I didn't actually have an example one for these hooks (got confused with a different hook). So I wrote one.
Attachment #382091 - Attachment is obsolete: true
Attachment #384369 - Flags: review?(mkanat)

Comment 4

9 years ago
Comment on attachment 384369 [details] [diff] [review]
v2

I love your example, but it's kind of important that the example plugin be incapable of modifying the database. Also, it's nice to put the word "Example" in the output somewhere so that people know that it came from the plugin.
Attachment #384369 - Flags: review?(mkanat) → review-
(Assignee)

Comment 5

9 years ago
I guess I can have a repair that just prints something....
(Assignee)

Comment 6

9 years ago
Created attachment 394711 [details] [diff] [review]
v3
Attachment #384369 - Attachment is obsolete: true
Attachment #394711 - Flags: review?(mkanat)

Updated

9 years ago
Attachment #394711 - Flags: review?(mkanat) → review+

Comment 7

9 years ago
Comment on attachment 394711 [details] [diff] [review]
v3

This looks good, but I think "checks" should become "check" on checkin (because "repair" is also singular).

Comment 8

9 years ago
I think this hook is too invasive to land on the branch.
Severity: normal → enhancement
Flags: approval+
OS: Linux → All
Hardware: x86 → All
Target Milestone: --- → Bugzilla 3.6

Comment 9

9 years ago
bbaetz, please commit your patch.
Status: NEW → ASSIGNED

Comment 10

9 years ago
I did the fix on checkin. I'll attach a new patch that's exactly what was checked in.

Checking in sanitycheck.cgi;
/cvsroot/mozilla/webtools/bugzilla/sanitycheck.cgi,v  <--  sanitycheck.cgi
new revision: 1.146; previous revision: 1.145
done
Checking in Bugzilla/Hook.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Hook.pm,v  <--  Hook.pm
new revision: 1.30; previous revision: 1.29
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/extensions/example/code/sanitycheck-check.pl,v
done
Checking in extensions/example/code/sanitycheck-check.pl;
/cvsroot/mozilla/webtools/bugzilla/extensions/example/code/sanitycheck-check.pl,v  <--  sanitycheck-check.pl
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/extensions/example/code/sanitycheck-repair.pl,v
done
Checking in extensions/example/code/sanitycheck-repair.pl;
/cvsroot/mozilla/webtools/bugzilla/extensions/example/code/sanitycheck-repair.pl,v  <--  sanitycheck-repair.pl
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/extensions/example/template/en/admin/sanitycheck/messages-statuses.html.tmpl,v
done
Checking in extensions/example/template/en/admin/sanitycheck/messages-statuses.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/extensions/example/template/en/admin/sanitycheck/messages-statuses.html.tmpl,v  <--  messages-statuses.html.tmpl
initial revision: 1.1
done
Checking in template/en/default/admin/sanitycheck/messages.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/sanitycheck/messages.html.tmpl,v  <--  messages.html.tmpl
new revision: 1.11; previous revision: 1.10
done
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 11

9 years ago
Created attachment 401952 [details] [diff] [review]
Checked-In Version
Attachment #394711 - Attachment is obsolete: true
Attachment #401952 - Flags: review+
You need to log in before you can comment on or make changes to this bug.