Last Comment Bug 691411 - Move js/src/xpconnect to js/xpconnect
: Move js/src/xpconnect to js/xpconnect
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Bobby Holley (busy)
:
Mentors:
: 307281 (view as bug list)
Depends on:
Blocks: 688012
  Show dependency treegraph
 
Reported: 2011-10-03 10:51 PDT by Bobby Holley (busy)
Modified: 2011-12-26 12:18 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
part 1 - Remove various dead directories in XPConnect. v1 (208.51 KB, patch)
2011-10-03 14:18 PDT, Bobby Holley (busy)
khuey: review+
Details | Diff | Review
part 2 - Move js/src/xpconnect to js/xpconnect and rename source files while we're at it. v1 (105.77 KB, patch)
2011-10-03 14:23 PDT, Bobby Holley (busy)
khuey: review+
mrbkap: superreview+
Details | Diff | Review

Description Bobby Holley (busy) 2011-10-03 10:51:08 PDT
Spun off from bug 688012 - this is long-overdue.
Comment 1 Bobby Holley (busy) 2011-10-03 14:18:58 PDT
Created attachment 564328 [details] [diff] [review]
part 1 - Remove various dead directories in XPConnect. v1

The first patch here just gets rid of some dead code. Flagging khuey for review, since I know how much he likes negative diffs. ;-)
Comment 2 Bobby Holley (busy) 2011-10-03 14:23:39 PDT
Created attachment 564333 [details] [diff] [review]
part 2 - Move js/src/xpconnect to js/xpconnect and rename source files while we're at it. v1

This patch does the move.

We've also been wanting to capitalize the source files in XPConnect, but haven't been able to because, among other things, pure recasings break on windows. Since we're already moving these files, this is an excellent opportunity to recase them as well.

Flagging khuey for the nitty-gritty details. Flagging mrbkap for sr to get his stamp of approval on the whole operation.

I plan to lands the changes here along with the ones in bug 688012 to minimize the disruption in peoples' patch queues.
Comment 3 Johnny Stenback (:jst, jst@mozilla.com) 2011-10-04 22:37:48 PDT
This will conflict pretty badly with peterv's nodelist binding changes in bug 648801. The changes there are rather large, so it would probably make sense to land this after bug 648801. And the changes there should land very soon now.
Comment 4 Bobby Holley (busy) 2011-10-04 23:01:58 PDT
(In reply to Johnny Stenback (:jst, jst@mozilla.com) from comment #3)
> This will conflict pretty badly with peterv's nodelist binding changes in
> bug 648801. The changes there are rather large, so it would probably make
> sense to land this after bug 648801. And the changes there should land very
> soon now.

Sure.

I think the patches can still be reviewed by khuey. We can hold off on blake's sr until peterv's stuff lands.
Comment 5 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-10-06 08:14:59 PDT
Comment on attachment 564328 [details] [diff] [review]
part 1 - Remove various dead directories in XPConnect. v1

I didn't look very closely at this.
Comment 6 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-10-06 08:25:21 PDT
Comment on attachment 564333 [details] [diff] [review]
part 2 - Move js/src/xpconnect to js/xpconnect and rename source files while we're at it. v1

Review of attachment 564333 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine, but perhaps we should consider condensing things even more (e.g. merging public/ and src/, etc).

::: dom/base/Makefile.in
@@ +136,5 @@
>  include $(topsrcdir)/config/rules.mk
>  
>  LOCAL_INCLUDES += \
> +		-I$(srcdir)/../../js/xpconnect/src \
> +		-I$(srcdir)/../../js/xpconnect/wrappers \

Since we're here anyways, use $(topsrcdir)?
Comment 7 Blake Kaplan (:mrbkap) (in and out until 7-14) 2011-10-06 13:36:46 PDT
Comment on attachment 564333 [details] [diff] [review]
part 2 - Move js/src/xpconnect to js/xpconnect and rename source files while we're at it. v1

This is going to hurt my muscle memory for a while. Let's do it.
Comment 8 Bobby Holley (busy) 2011-10-14 16:53:50 PDT
This landed to mozilla-central and appears to have stuck:

http://hg.mozilla.org/mozilla-central/rev/80d7e83b7659
http://hg.mozilla.org/mozilla-central/rev/51541b56d20d

It also required a followup fix for the dom_quickstubs.h dependency, which caused a race condition where the build might fail (but never on try :\ ):

http://hg.mozilla.org/mozilla-central/rev/6d5fd5a30c71

Resolving fixed.
Comment 9 :Ms2ger 2011-12-26 12:18:41 PST
*** Bug 307281 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.