Closed Bug 382647 Opened 17 years ago Closed 17 years ago

Move xpfe Bookmarks source code to /suite


(SeaMonkey :: Bookmarks & History, defect)

Not set


(Not tracked)



(Reporter: standard8, Assigned: standard8)




(2 files)

Now that bug 378585 is fixed, we can move the bookmarks source code to /suite (Robert has already moved the chrome).

The logical place I think would be under /suite/browser, but chrome is already in suite/common.

So do we put the src under suite/browser/bookmarks and move the chrome across and update the uris (communicator->navigator)? Or do we just put the src with the chrome under suite/common/bookmarks/?

btw do we want suite/<whatever>/bookmarks/{public,src} directories as well?
I'm not sure the public/src is the way to go, bsmedberg might be able to better tell that.
I think we should get browser-specific code into suite/browser (bookmarks and history, not sure what else we have).
Depends on: 389070
Attached patch cvs movesSplinter Review
Now that bookmarks is linking to external API, we can move it to /suite :-)

I'm putting this in suite/browser just like we did with the search code.

Here's the moves file, patch coming up in a mo.
Attachment #280269 - Flags: superreview?(neil)
Attachment #280269 - Flags: review?(kairo)
Attached patch Build patchSplinter Review
Attachment #280271 - Flags: superreview?(neil)
Attachment #280271 - Flags: review?(kairo)
Comment on attachment 280269 [details] [diff] [review]
cvs moves

The file locations look good, r=me :)
Attachment #280269 - Flags: review?(kairo) → review+
Attachment #280269 - Flags: superreview?(neil) → superreview+
Comment on attachment 280271 [details] [diff] [review]
Build patch

>Index: suite/browser/src/
>RCS file: /cvsroot/mozilla/suite/browser/src/,v
>retrieving revision 1.10
>diff -u -p -r1.10
>--- suite/browser/src/	7 Sep 2007 14:25:03 -0000	1.10
>+++ suite/browser/src/	9 Sep 2007 20:57:13 -0000
>@@ -57,14 +57,18 @@ REQUIRES = \
> 	gfx \
> 	layout \
> 	necko \
>+	nkcache \
> 	pref \
> 	rdf \
> 	uconv \
> 	intl \
>-	appcomps \
>+	txmgr \
>+	chardet \
>+	locale \
> 	$(NULL)

>Index: suite/build/
>RCS file: /cvsroot/mozilla/suite/build/,v
>retrieving revision 1.6
>diff -u -p -r1.6
>--- suite/build/	7 Sep 2007 14:25:04 -0000	1.6
>+++ suite/build/	9 Sep 2007 20:57:13 -0000
>@@ -62,7 +62,9 @@ REQUIRES	= \
> 		windowwatcher \
> 		suitebrowser \
> 		suitemigration \
>-		appcomps \
>+		nkcache \
>+		txmgr \
>+		chardet \
> 		$(NULL)
There's some inconsistencies creeping in here. I don't mind either way, but the first is that src/ was almost alphabetical, and you added nkcache alphabetically but the other three you simply appended. The second is that one file requires locale and the other doesn't, although I guess that's because it's only used by nsBookmarksService.cpp and not nsBookmarksService.h?
Attachment #280271 - Flags: superreview?(neil) → superreview+
Depends on: 395662
Comment on attachment 280271 [details] [diff] [review]
Build patch

yes, I like it, let's go for it. not referencing xpfe/ any more is really nice ;-)

Attachment #280271 - Flags: review?(kairo) → review+
Comment on attachment 280271 [details] [diff] [review]
Build patch

Requesting approval for checking in of the and xpfe/components/bookmarks changes.

This patch is all about moving code that's NPOTFFB away from xpfe into suite, but because of the current tree state, approval has to be requested for the build file changes where FF does use them.
Attachment #280271 - Flags: approval1.9?
I've checked in the suite specific parts of this. I'm waiting on approvals or tree going a different colour before checking in the xpfe and toolkit parts.
Comment on attachment 280271 [details] [diff] [review]
Build patch

I had a talk on IRC today, and got a=moconnor there for the hunk of this patch.
Comment on attachment 280271 [details] [diff] [review]
Build patch

Cancelling review request per comment 9 as its been received over irc.
Attachment #280271 - Flags: approval1.9?
All patches checked in -> fixed.
Closed: 17 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0alpha
You need to log in before you can comment on or make changes to this bug.