Move xpfe Bookmarks source code to /suite

RESOLVED FIXED in seamonkey2.0a1

Status

RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

Trunk
seamonkey2.0a1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

12 years ago
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?

Comment 1

12 years ago
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).
(Assignee)

Updated

11 years ago
Depends on: 389070
(Assignee)

Comment 2

11 years ago
Created attachment 280269 [details] [diff] [review]
cvs moves

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)
(Assignee)

Comment 3

11 years ago
Created attachment 280271 [details] [diff] [review]
Build patch
Attachment #280271 - Flags: superreview?(neil)
Attachment #280271 - Flags: review?(kairo)

Comment 4

11 years ago
Comment on attachment 280269 [details] [diff] [review]
cvs moves

The file locations look good, r=me :)
Attachment #280269 - Flags: review?(kairo) → review+

Updated

11 years ago
Attachment #280269 - Flags: superreview?(neil) → superreview+

Comment 5

11 years ago
Comment on attachment 280271 [details] [diff] [review]
Build patch

>Index: suite/browser/src/Makefile.in
>===================================================================
>RCS file: /cvsroot/mozilla/suite/browser/src/Makefile.in,v
>retrieving revision 1.10
>diff -u -p -r1.10 Makefile.in
>--- suite/browser/src/Makefile.in	7 Sep 2007 14:25:03 -0000	1.10
>+++ suite/browser/src/Makefile.in	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/Makefile.in
>===================================================================
>RCS file: /cvsroot/mozilla/suite/build/Makefile.in,v
>retrieving revision 1.6
>diff -u -p -r1.6 Makefile.in
>--- suite/build/Makefile.in	7 Sep 2007 14:25:04 -0000	1.6
>+++ suite/build/Makefile.in	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/Makefile.in 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+
(Assignee)

Updated

11 years ago
Depends on: 395662

Comment 6

11 years ago
Comment on attachment 280271 [details] [diff] [review]
Build patch

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

r=me
Attachment #280271 - Flags: review?(kairo) → review+
(Assignee)

Comment 7

11 years ago
Comment on attachment 280271 [details] [diff] [review]
Build patch

Requesting approval for checking in of the toolkit-makefiles.sh 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?
(Assignee)

Comment 8

11 years ago
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 9

11 years ago
Comment on attachment 280271 [details] [diff] [review]
Build patch

I had a talk on IRC today, and got a=moconnor there for the toolkit-makefiles.sh hunk of this patch.
(Assignee)

Comment 10

11 years ago
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?
(Assignee)

Comment 11

11 years ago
All patches checked in -> fixed.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Target Milestone: --- → seamonkey2.0alpha
You need to log in before you can comment on or make changes to this bug.