Closed
Bug 277502
Opened 21 years ago
Closed 19 years ago
Re-organize checksetup.pl to be understood more easily (make it a short series of subroutines)
Categories
(Bugzilla :: Installation & Upgrading, enhancement, P3)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.0
People
(Reporter: mkanat, Assigned: mkanat)
References
(Blocks 1 open bug)
Details
(Keywords: meta)
Attachments
(2 files)
8.85 KB,
text/plain
|
Details | |
19.26 KB,
patch
|
mkanat
:
review+
|
Details | Diff | Splinter Review |
The other morning I was thinking about how to make checksetup more logical and
easy to understand without risking major code changes (since checksetup is hard
to test when we change things).
Basically, I think that the basic code of checksetup.pl should look like the
classic main() function in an application, and just be a simple series of
subroutines. Something like:
check_versions()
check_schema()
create_admin()
check_sanity()
Or something like that. I'll have to investigate checksetup a bit more to get
the actual list of functions that it will consist of.
It will also make it a lot easier to futher improve the checksetup code, and
move some things out of that script that don't need to be there.
Assignee | ||
Comment 1•21 years ago
|
||
I just spent a few hours making this. It's an English description,
step-by-step, of what checksetup does, in detail.
This should help a lot with any re-organization.
Assignee | ||
Updated•21 years ago
|
Blocks: bz-majorarch
Assignee | ||
Updated•21 years ago
|
Comment 2•21 years ago
|
||
Attchment 170860 looks like the right idea. I suggest that each line (numbered
item) also become a subroutine as well as having a subroutine for each major
category.
Assignee | ||
Comment 3•21 years ago
|
||
(In reply to comment #2)
> Attchment 170860 looks like the right idea. I suggest that each line (numbered
> item) also become a subroutine as well as having a subroutine for each major
> category.
Exactly my plan. :-)
Assignee | ||
Comment 4•21 years ago
|
||
So, I'm thinking that as I do this, I might move these subroutines into perl
modules, and checksetup would just call them:
Bugzilla::Requirements
Bugzilla::Install
Bugzilla::Requirements would contain everything that's in the BEGIN block, now.
This would also allow other code (like runtests) to check that the modules exist.
Assignee | ||
Updated•20 years ago
|
Priority: -- → P3
Assignee | ||
Comment 5•20 years ago
|
||
I'm going to start some work on this in 2.22, but will probably not finish until
2.24.
Target Milestone: --- → Bugzilla 2.24
And please, don't forget a routine (or, better, pluggable mechanism with
overrides/plug-ins) to check bz_schema against the real DB. Then we'll be able
to remove that wierd comment in localconfig for db_check parameter. I mean a
phrase about the "moon phase". :)
I've ran into it last week, so had to drop all the problem tables along with
bz_schema itself as a workaround. Thanks God, it was just a test DB.
Assignee | ||
Comment 7•20 years ago
|
||
(In reply to comment #6)
> And please, don't forget a routine (or, better, pluggable mechanism with
> overrides/plug-ins) to check bz_schema against the real DB. Then we'll be able
> to remove that wierd comment in localconfig for db_check parameter. I mean a
> phrase about the "moon phase". :)
Hahahaha. Actually, that "moon phase" comment is totally unrelated to that
part of checksetup. The "moon phase" stuff is involved in checking the DB
version numbers and so forth.
> I've ran into it last week, so had to drop all the problem tables along with
> bz_schema itself as a workaround. Thanks God, it was just a test DB.
That's hopefully also addressed by bug 299230.
(In reply to comment #7)
> (In reply to comment #6)
> > And please, don't forget a routine (or, better, pluggable mechanism with
> > overrides/plug-ins) to check bz_schema against the real DB. Then we'll be able
> > to remove that wierd comment in localconfig for db_check parameter. I mean a
> > phrase about the "moon phase". :)
>
> Hahahaha. Actually, that "moon phase" comment is totally unrelated to that
> part of checksetup. The "moon phase" stuff is involved in checking the DB
> version numbers and so forth.
Well, so we also have, say, "Venus phases" in there :)
> > I've ran into it last week, so had to drop all the problem tables along with
> > bz_schema itself as a workaround. Thanks God, it was just a test DB.
>
> That's hopefully also addressed by bug 299230.
No, that's not the case. I've been forced to drop the tables (or manually check
for correctness) to make their structure consistent with schema description.
Though I did not wish to.
Should I fill in the bug on this and add it into blockers of this one?
Assignee | ||
Comment 9•20 years ago
|
||
(In reply to comment #8)
> Should I fill in the bug on this and add it into blockers of this one?
Well, actually, our whole discussion here is really unrelated to this bug. So
anything we're talking about should go into another bug. Actually *changing* the
way checksetup works would be a totally separate bug from this one, and
certainly any problems you're experiencing with checksetup would be a different
bug from this one. But yes, I'd certainly appreciate a bug report with the
details of the situation. :-)
Updated•19 years ago
|
QA Contact: mattyt-bugzilla → default-qa
Assignee | ||
Updated•19 years ago
|
Alias: bz-checksetup
Assignee | ||
Comment 11•19 years ago
|
||
Okay, here's the very final step of this bug, this final cleanup of checksetup.pl.
You'll notice that there's now a very nice "How Checksetup Works" section in the POD. It should be useful to developers especially.
Granting myself review, as module owner.
Attachment #238349 -
Flags: review+
Assignee | ||
Comment 12•19 years ago
|
||
Requesting approval directly, as module owner.
Status: NEW → ASSIGNED
Flags: approval?
Updated•19 years ago
|
Flags: approval? → approval+
Assignee | ||
Comment 13•19 years ago
|
||
*Cheering can be heard!*
Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v <-- checksetup.pl
new revision: 1.543; previous revision: 1.542
done
Checking in Bugzilla/Install.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install.pm,v <-- Install.pm
new revision: 1.8; previous revision: 1.7
done
Checking in Bugzilla/Install/Requirements.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/Requirements.pm,v <-- Requirements.pm
new revision: 1.18; previous revision: 1.17
done
Checking in template/en/default/global/messages.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/messages.html.tmpl,v <-- messages.html.tmpl
new revision: 1.43; previous revision: 1.42
don
Alias: bz-checksetup
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•19 years ago
|
Blocks: bz-webinstall
You need to log in
before you can comment on or make changes to this bug.
Description
•