Closed
Bug 192516
Opened 22 years ago
Closed 21 years ago
Move loose .pm files into the Bugzilla directory
Categories
(Bugzilla :: Bugzilla-General, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.18
People
(Reporter: justdave, Assigned: justdave)
References
Details
Attachments
(1 file, 2 obsolete files)
54.47 KB,
patch
|
myk
:
review+
gerv
:
review+
|
Details | Diff | Splinter Review |
The following files belong in the Bugzilla directory, and the internal packages
should be renamed appropriately:
Attachment.pm
Bug.pm
RelationSet.pm
Token.pm
All references to these packages from everywhere else in Bugzilla would need to
be modified appropriately.
Assignee | ||
Comment 1•22 years ago
|
||
And yes, I'm aware we'll lose cvs history. Oh Well. :-P
If it'd help, we can put comments at the top of the files indicating where the
original location was in CVS to aid anyone who wants to get a more accurate
cvsblame :)
Comment 2•22 years ago
|
||
Instead of putting comments at the top of the files (ick), put them in the first
CVS comment. They are metadata.
But can't we get leaf or someone to do a repository move? A lot of projects have
an admin do that when files need shifting about.
Also, should we move Bugzilla.pm? use Bugzilla::Bugzilla looks a bit odd. Maybe
it needs renaming to Instance.pm at the same time ;-) Or we could leave it where
it is; Bugzilla->dbh is shorter and nicer than Bugzilla::Instance->dbh or
Bugzilla::Bugzilla->dbh.
Gerv
Comment 3•22 years ago
|
||
Bugzilla.pm was meant to stay there.
I was going to avoid moving all the rest until they were rewritten.
At some point, we should move all of these into lib/*, but I'mnot sure how
newinstall is working with that stuff.
Assignee | ||
Comment 4•22 years ago
|
||
Yeah, that's why I left Bugzilla.pm off the list, the way the structure is set
up, it makes better sense right where it is. That, or as Brad notes, it and the
Bugzilla directory both getting moved inside a lib/ directory.
I actually have access to move files in the repository now (I just nuked those
three directories the other day), not that I'm incredibly fond of doing so.
Note that moving the file in the respository means someone doing a checkout of
the 2.16 branch will wind up with the file in the wrong place because the tags
match (even if we copy it, then they'll just get a copy in both places). Better
to delete/readd and follow the cvs comments to find the original if we need to.
Assignee | ||
Comment 5•22 years ago
|
||
Assignee | ||
Comment 6•22 years ago
|
||
The "pseudo-patch" was created with the files in their original locations, to
more easily show the actual changes that were made to the .pm files that were
moved.
Comment 7•22 years ago
|
||
Comment on attachment 116687 [details] [diff] [review]
Pseudo-patch for review purposes
Can we get the best of both worlds? Instead of doing a repository move, do a
repository copy. So, the CVS history appears both in the new locations and the
old locations. I know this is vaguely inconsistent (having the same history in
two places) but, unlike the templates we moved, these files have significant
CVS history, and being able to access that directly through Bonsai would be a
great help. And it avoids the 2.16 problem, because the files are still in
their original locations too (albeit CVS removed on the trunk.)
r=gerv on the patch. I think I can trust Dave to add "Bugzilla::" to a lot of
package names. :-) (Are there any changes other than that?)
Gerv
Attachment #116687 -
Flags: review+
Assignee | ||
Comment 8•22 years ago
|
||
If you copy the file in the repository, that means if someone checks out 2.16,
they'll get both Bug.pm and Bugzilla/Bug.pm, unless someone spends a lot of time
manually deleting old tags off of Bugzilla/Bug.pm... (and I don't think you can
delete a branch tag).
Assignee | ||
Updated•22 years ago
|
Attachment #116687 -
Flags: review?(bbaetz)
Assignee | ||
Comment 9•22 years ago
|
||
Comment on attachment 116687 [details] [diff] [review]
Pseudo-patch for review purposes
This conflicts with the Auth.pm patch. Let's land that one first....
Attachment #116687 -
Attachment is obsolete: true
Attachment #116687 -
Flags: review?(bbaetz)
Assignee | ||
Updated•22 years ago
|
Attachment #116681 -
Attachment is obsolete: true
Comment 10•22 years ago
|
||
Does it matter if they get both, as long as no code refers to Bugzilla/Bug.pm,
which it won't?
I'd say that was a smaller downside than making the cvs history of all these
modules much harder to find.
Gerv
Assignee | ||
Comment 11•22 years ago
|
||
see also bug 105614 dupe, or dependency, or? Should we do both of these at once?
Bug 105614 would have us creating a lib directory into which Bugzilla.pm as well
as the Bugzilla folder would go.
I think I'm sold on the idea of copying the files within the repository, then
cvs-deleting the old one. We can go through and manually remove all the old
tags on the files in the new location, which will help when someone pulls
-rBUGZILLA-2_16_2 or -rBugzilla_Stable for example, but they'll still get them
if they pull by date or a branch name.
Comment 12•22 years ago
|
||
Personally, I don't think we should do bug 105614. It seems like a lot of
upheaval for very little gain. I like our current directory structure.
Gerv
Assignee | ||
Comment 13•21 years ago
|
||
OK, lets do this and get it over with. With all the new packages we've added in
2.17.x and the new object-oriented nature of almost everything, this would be a
good time to have this happen, before 2.18 releases.
I have lots of experience doing "cvs moves" for Mozilla now, and I no longer
have any fear of the issues raised in comments 4 and 8 (I know how to eleviate
90% of those issues on a cvs move now :)
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 2.18
Assignee | ||
Comment 14•21 years ago
|
||
CVS *copy* has already been completed (and can be easily reversed if we wind up
not doing this, but I think we should).
This patch will cvs remove the 4 files from the old location, and makes changes
to them in their new location to make them fit there.
Assignee | ||
Updated•21 years ago
|
Attachment #144100 -
Flags: review?(myk)
Assignee | ||
Updated•21 years ago
|
Attachment #144100 -
Flags: review?(bbaetz)
Assignee | ||
Updated•21 years ago
|
Attachment #144100 -
Flags: review?(gerv)
Comment 15•21 years ago
|
||
Comment on attachment 144100 [details] [diff] [review]
Patch v2
Looks good, seems to work per some basic testing. r=myk
Attachment #144100 -
Flags: review?(myk) → review+
Comment 16•21 years ago
|
||
Comment on attachment 144100 [details] [diff] [review]
Patch v2
This looks fine to me. It's one of those things where, if you get it wrong,
it's really obvious quite quickly, so if Myk has tested it, I say go for it.
r=gerv.
Gerv
Attachment #144100 -
Flags: review?(gerv)
Assignee | ||
Comment 17•21 years ago
|
||
Removing Attachment.pm;
/cvsroot/mozilla/webtools/bugzilla/Attachment.pm,v <-- Attachment.pm
new revision: delete; previous revision: 1.15
done
Removing Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bug.pm,v <-- Bug.pm
new revision: delete; previous revision: 1.33
done
Removing RelationSet.pm;
/cvsroot/mozilla/webtools/bugzilla/RelationSet.pm,v <-- RelationSet.pm
new revision: delete; previous revision: 1.9
done
Removing Token.pm;
/cvsroot/mozilla/webtools/bugzilla/Token.pm,v <-- Token.pm
new revision: delete; previous revision: 1.20
done
Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v <-- checksetup.pl
new revision: 1.268; previous revision: 1.267
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v <-- globals.pl
new revision: 1.257; previous revision: 1.256
done
Checking in move.pl;
/cvsroot/mozilla/webtools/bugzilla/move.pl,v <-- move.pl
new revision: 1.24; previous revision: 1.23
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v <-- post_bug.cgi
new revision: 1.87; previous revision: 1.86
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v <-- process_bug.cgi
new revision: 1.200; previous revision: 1.199
done
Checking in show_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/show_bug.cgi,v <-- show_bug.cgi
new revision: 1.26; previous revision: 1.25
done
Checking in token.cgi;
/cvsroot/mozilla/webtools/bugzilla/token.cgi,v <-- token.cgi
new revision: 1.22; previous revision: 1.21
done
Checking in userprefs.cgi;
/cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v <-- userprefs.cgi
new revision: 1.55; previous revision: 1.54
done
Checking in Bugzilla/Attachment.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Attachment.pm,v <-- Attachment.pm
new revision: 1.16; previous revision: 1.15
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm
new revision: 1.34; previous revision: 1.33
done
Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v <-- BugMail.pm
new revision: 1.10; previous revision: 1.9
done
Checking in Bugzilla/Flag.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v <-- Flag.pm
new revision: 1.16; previous revision: 1.15
done
Checking in Bugzilla/RelationSet.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/RelationSet.pm,v <-- RelationSet.pm
new revision: 1.10; previous revision: 1.9
done
Checking in Bugzilla/Token.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Token.pm,v <-- Token.pm
new revision: 1.21; previous revision: 1.20
done
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Flags: approval+
Resolution: --- → FIXED
Assignee | ||
Updated•21 years ago
|
Attachment #144100 -
Flags: review?(bbaetz)
Updated•12 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•