Bug 49586 - Chrome DevTools: concatenate CSS files, do not link missing JS files in release mode.
Summary: Chrome DevTools: concatenate CSS files, do not link missing JS files in relea...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-16 02:43 PST by Pavel Feldman
Modified: 2010-11-16 08:44 PST (History)
5 users (show)

See Also:

Attachments
[PATCH] Proposed change. (13.20 KB, patch)
2010-11-16 03:09 PST, Pavel Feldman 		no flags Details | Formatted Diff | Diff
[PATCH] Same with style fixes (13.58 KB, patch)
2010-11-16 05:22 PST, Pavel Feldman 		no flags Details | Formatted Diff | Diff
[PATCH] Same with style fixes for real. (13.58 KB, patch)
2010-11-16 05:30 PST, Pavel Feldman 		no flags Details | Formatted Diff | Diff
[PATCH] Addressed Mikhail's comments. (13.50 KB, patch)
2010-11-16 05:38 PST, Pavel Feldman 		yurys: review+
Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2010-11-16 02:43:10 PST 
Drive-by move of py script files upstream.
Comment 1 Pavel Feldman 2010-11-16 03:09:03 PST 
Created attachment 73976 [details]
[PATCH] Proposed change.
Comment 2 WebKit Review Bot 2010-11-16 03:12:09 PST 
Attachment 73976 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebKit/chromium/Build/concatenate_css_files.py', u'WebKit/chromium/Build/concatenate_js_files.py', u'WebKit/chromium/Build/generate_devtools_html.py', u'WebKit/chromium/ChangeLog', u'WebKit/chromium/WebKit.gyp']" exit_code: 1
Last 3072 characters of output:
 four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:74:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:75:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:76:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:77:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:81:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:86:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:9:  expected 2 blank lines, found 1  [pep8/E302] [5]
WebKit/chromium/Build/generate_devtools_html.py:10:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:11:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:13:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:15:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:16:  at least two spaces before inline comment  [pep8/E261] [5]
WebKit/chromium/Build/generate_devtools_html.py:21:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:29:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:30:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:31:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:32:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:33:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:35:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:37:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:39:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:41:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:43:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:47:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:52:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:53:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:57:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/generate_devtools_html.py:62:  indentation is not a multiple of four  [pep8/E111] [5]
Total errors found: 79 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Mikhail Naganov 2010-11-16 04:49:09 PST 
Comment on attachment 73976 [details]
[PATCH] Proposed change.

View in context: https://bugs.webkit.org/attachment.cgi?id=73976&action=review

> WebKit/chromium/WebKit.gyp:733
>                      '<@(devtools_files)',

If you are adding 'DevTools.js' and 'Tests.js' manually in the script, why do you need devtools_files here?

> WebKit/chromium/WebKit.gyp:734
> +                    '<(SHARED_INTERMEDIATE_DIR)/webcore/InspectorBackendStub.js'

BTW, Python is OK with array elements list ended with a comma.

> WebKit/chromium/WebKit.gyp:750
> +                '../../WebCore/WebCore.gyp/WebCore.gyp:inspector_protocol_sources'

Why CSS depends on inspector_protocol_sources?

> WebKit/chromium/WebKit.gyp:761
> +                    '<@(devtools_files)'

If you are adding 'devTools.css' manually in the script, why do you need devtools_files here?
Comment 4 Pavel Feldman 2010-11-16 05:22:14 PST 
Created attachment 73984 [details]
[PATCH] Same with style fixes
Comment 5 WebKit Review Bot 2010-11-16 05:23:51 PST 
Attachment 73984 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebKit/chromium/Build/concatenate_css_files.py', u'WebKit/chromium/Build/concatenate_js_files.py', u'WebKit/chromium/Build/generate_devtools_html.py', u'WebKit/chromium/ChangeLog', u'WebKit/chromium/WebKit.gyp']" exit_code: 1
Last 3072 characters of output:
2] [5]
WebKit/chromium/Build/concatenate_js_files.py:31:  multiple statements on one line (semicolon)  [pep8/E702] [5]
WebKit/chromium/Build/concatenate_js_files.py:46:  expected 2 blank lines, found 1  [pep8/E302] [5]
WebKit/chromium/Build/concatenate_js_files.py:73:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_js_files.py:78:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_css_files.py:14:  expected 2 blank lines, found 1  [pep8/E302] [5]
WebKit/chromium/Build/concatenate_css_files.py:21:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_css_files.py:29:  expected 2 blank lines, found 1  [pep8/E302] [5]
WebKit/chromium/Build/concatenate_css_files.py:31:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:32:  multiple statements on one line (semicolon)  [pep8/E702] [5]
WebKit/chromium/Build/concatenate_css_files.py:32:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:34:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:35:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:36:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:37:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:38:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:39:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:40:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:41:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:43:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:44:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:45:  indentation is not a multiple of four  [pep8/E111] [5]
WebKit/chromium/Build/concatenate_css_files.py:47:  expected 2 blank lines, found 1  [pep8/E302] [5]
WebKit/chromium/Build/concatenate_css_files.py:53:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_css_files.py:58:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_css_files.py:61:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_css_files.py:73:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_css_files.py:78:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/concatenate_css_files.py:84:  trailing whitespace  [pep8/W291] [5]
WebKit/chromium/Build/generate_devtools_html.py:9:  expected 2 blank lines, found 1  [pep8/E302] [5]
WebKit/chromium/Build/generate_devtools_html.py:16:  at least two spaces before inline comment  [pep8/E261] [5]
Total errors found: 32 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Pavel Feldman 2010-11-16 05:30:10 PST 
Created attachment 73985 [details]
[PATCH] Same with style fixes for real.
Comment 7 Pavel Feldman 2010-11-16 05:37:11 PST 
> > WebKit/chromium/WebKit.gyp:750
> > +                '../../WebCore/WebCore.gyp/WebCore.gyp:inspector_protocol_sources'
> 

Fixed.

Rest seems to be correct as we discussed offline.
Comment 8 Pavel Feldman 2010-11-16 05:38:04 PST 
Created attachment 73986 [details]
[PATCH] Addressed Mikhail's comments.
Comment 9 Mikhail Naganov 2010-11-16 05:42:11 PST 
Comment on attachment 73986 [details]
[PATCH] Addressed Mikhail's comments.

Looks good to me, I would put r+
Comment 10 Yury Semikhatsky 2010-11-16 08:16:57 PST 
Comment on attachment 73986 [details]
[PATCH] Addressed Mikhail's comments.

View in context: https://bugs.webkit.org/attachment.cgi?id=73986&action=review

Please address the comments before landing.

> WebKit/chromium/Build/concatenate_css_files.py:2
> +# Copyright (c) 2009 The Chromium Authors. All rights reserved.

it's 2010 already:) and btw should we use a different license header like in the other webkit files?

> WebKit/chromium/Build/concatenate_css_files.py:7
> +# using <script> tags in a given 'order.html' file.

did you mean using <link> tags?

> WebKit/chromium/Build/concatenate_css_files.py:60
> +    extractor.ordered_css_files.append('devTools.css')

should we have a comment explaining this stuff?

> WebKit/chromium/Build/concatenate_css_files.py:67
> +        if (full_path is None):

nit: if not full_path:

> WebKit/chromium/Build/concatenate_js_files.py:2
> +# Copyright (c) 2009 The Chromium Authors. All rights reserved.

2009->2010

> WebKit/chromium/Build/concatenate_js_files.py:42
> +                    raise Exception('Ambiguous file %s: found in %s and %s' %

I understand that these are build scripts but can we extract common functionality into its own file?

> WebKit/chromium/Build/generate_devtools_html.py:2
> +# Copyright (c) 2009 The Chromium Authors. All rights reserved.

2009->2010
Comment 11 Pavel Feldman 2010-11-16 08:44:36 PST 
Committing to http://svn.webkit.org/repository/webkit/trunk ...
        A       WebKit/chromium/Build/concatenate_css_files.py
        A       WebKit/chromium/Build/concatenate_js_files.py
        A       WebKit/chromium/Build/generate_devtools_html.py
        M       WebKit/chromium/ChangeLog
        M       WebKit/chromium/WebKit.gyp
Committed r72103