Closed Bug 192516 Opened 22 years ago Closed 21 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: 21 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: