Closed Bug 192516 Opened 21 years ago Closed 20 years ago

Move loose .pm files into the Bugzilla directory

Categories

(Bugzilla :: Bugzilla-General, defect)

2.17.3
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: justdave, Assigned: justdave)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
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 :)
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
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.
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.
Attached patch Patch (obsolete) — Splinter Review
Attached patch Pseudo-patch for review purposes (obsolete) — Splinter Review
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 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+
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).
Attachment #116687 - Flags: review?(bbaetz)
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)
Attachment #116681 - Attachment is obsolete: true
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
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.
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
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
Attached patch Patch v2Splinter Review
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.
Attachment #144100 - Flags: review?(myk)
Attachment #144100 - Flags: review?(bbaetz)
Attachment #144100 - Flags: review?(gerv)
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 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)
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: 20 years ago
Flags: approval+
Resolution: --- → FIXED
Attachment #144100 - Flags: review?(bbaetz)
Blocks: 237864
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: