Last Comment Bug 702532 - Unnecessary preprocessing in the devtools jar manifest
: Unnecessary preprocessing in the devtools jar manifest
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: Trunk
: All All
-- enhancement (vote)
: Firefox 15
Assigned To: Cedric Vivier [:cedricv]
: J. Ryan Stinnett [:jryans] (use ni?)
Depends on:
  Show dependency treegraph
Reported: 2011-11-14 23:49 PST by Panos Astithas [:past]
Modified: 2012-06-01 05:50 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch v1 (3.72 KB, patch)
2012-02-12 08:00 PST, Cedric Vivier [:cedricv]
rcampbell: review+
Details | Diff | Splinter Review
Patch v2 (3.14 KB, patch)
2012-05-30 03:00 PDT, Panos Astithas [:past]
no flags Details | Diff | Splinter Review

Description User image Panos Astithas [:past] 2011-11-14 23:49:14 PST
From the 3 files that are marked for preprocessing in browser/devtools/, only scratchpad.xul looks like it needs it and that one could probably do without the #ifdef 0 to hide a comment block:

*   content/browser/inspector.html                (highlighter/inspector.html)
*   content/browser/scratchpad.xul                (scratchpad/scratchpad.xul)
*   content/browser/scratchpad.js                 (scratchpad/scratchpad.js)

Hopefully this would shorten build times by a tiny fraction, which might be non-trivial for tinderboxes.
Comment 1 User image Cedric Vivier [:cedricv] 2012-02-12 08:00:16 PST
Created attachment 596480 [details] [diff] [review]
patch v1

Let's do it ;)
More importantly it avoids surprises when using --enable-chrome-format=symlink.
Comment 2 User image Panos Astithas [:past] 2012-02-12 12:11:19 PST
I remember I had problems when I filed the bug getting a functional build after clobbering. Does this patch work after building from a clean slate? A try run should be sufficient, too.
Comment 3 User image Justin Dolske [:Dolske] 2012-04-15 18:25:35 PDT
Comment on attachment 596480 [details] [diff] [review]
patch v1

Unassigned review! Sending to... *spins wheel* dcamp!

Please review/reassign/close as appropriate.
Comment 4 User image Rob Campbell [:rc] (:robcee) 2012-04-16 05:28:04 PDT
Comment on attachment 596480 [details] [diff] [review]
patch v1

I dunno. Including that comment in our source file could really bloat the Firefox download size. R+ anyway.
Comment 5 User image Rob Campbell [:rc] (:robcee) 2012-04-16 05:28:24 PDT
(totally stole that review from dcamp. swish.)
Comment 6 User image Dave Camp (:dcamp) 2012-05-29 16:40:40 PDT
Is this ready to land?  Does it need rebasing?
Comment 7 User image Panos Astithas [:past] 2012-05-30 03:00:08 PDT
Created attachment 628288 [details] [diff] [review]
Patch v2

Updated patch to reflect current reality. Try run:
Comment 8 User image Rob Campbell [:rc] (:robcee) 2012-05-31 16:23:50 PDT
Comment 9 User image Rob Campbell [:rc] (:robcee) 2012-06-01 05:50:51 PDT

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