RESOLVED FIXED 31454
WebKit Coding Style Guidelines should say not to use "using namespace" statements in header files 
https://bugs.webkit.org/show_bug.cgi?id=31454
Summary WebKit Coding Style Guidelines should say not to use "using namespace" statem...
Chris Jerdonek
Reported 2009-11-12 20:28:01 PST
Created attachment 43130 [details] Patch submitted On the webkit-dev e-mail list, Darin Adler said that "using namespace" statements are not permitted in header files (see this report's URL for the e-mail). He said we should clarify that -- hence this report. A patch to the Coding Style Guidelines is attached. Thanks.
Attachments
Patch submitted (1.04 KB, patch)
2009-11-12 20:28 PST, Chris Jerdonek 		no flags
DetailsFormatted DiffDiff
New patch: added some closing </li>'s (1.25 KB, patch)
2009-11-12 23:06 PST, Chris Jerdonek 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Darin Adler
Comment 1 2009-11-12 21:19:47 PST
Comment on attachment 43130 [details] Patch submitted Need a </li>.
Chris Jerdonek
Comment 2 2009-11-12 23:06:23 PST
Created attachment 43136 [details] New patch: added some closing </li>'s The surrounding sections did not have closing </li>'s, which is what I was going by. (The entire document is inconsistent in this regard.) I am attaching a new patch with closing </li>'s for the three list items in the "using namespace" section. Thanks.
Chris Jerdonek
Comment 3 2009-11-12 23:08:22 PST
Comment on attachment 43136 [details] New patch: added some closing </li>'s Marking previous attachment as a patch
Darin Adler
Comment 4 2009-11-13 08:40:02 PST
Comment on attachment 43136 [details] New patch: added some closing </li>'s Someone should run the file through an HTML validator and fix issues they find. r=me on this change
Eric Seidel (no email)
Comment 5 2009-11-13 12:32:58 PST
Comment on attachment 43130 [details] Patch submitted Be sure to mark your old patches as obsolete when uploading replacement patches. Some tools, like bugzilla-tool post-diff or bugzilla-tool post-commits will do this automatically for you.
WebKit Commit Bot
Comment 6 2009-11-13 13:59:49 PST
Comment on attachment 43136 [details] New patch: added some closing </li>'s Rejecting patch 43136 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1 Running build-dumprendertree Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 11621 test cases. fast/workers/worker-terminate.html -> crashed Exiting early after 1 failures. 8405 tests run. 361.78s total testing time 8404 test cases (99%) succeeded 1 test case (<1%) crashed 3 test cases (<1%) had stderr output
Eric Seidel (no email)
Comment 7 2009-11-13 14:01:18 PST
Comment on attachment 43136 [details] New patch: added some closing </li>'s Not your fault. :( Bug 31461 again.
WebKit Commit Bot
Comment 8 2009-11-13 14:12:42 PST
Comment on attachment 43136 [details] New patch: added some closing </li>'s Rejecting patch 43136 from commit-queue. Failed to run "WebKitTools/Scripts/build-webkit" exit_code: 1 Last 500 characters of output: ld/WebCore.build/Release/WebCore.build/Objects-normal/i386/JSCompositionEvent.o /Users/eseidel/Projects/CommitQueue/WebKitBuild/Release/DerivedSources/WebCore/JSCompositionEvent.cpp normal i386 c++ com.apple.compilers.gcc.4_2 Distributed-CompileC /Users/eseidel/Projects/CommitQueue/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/i386/JSEventCustom.o /Users/eseidel/Projects/CommitQueue/WebCore/bindings/js/JSEventCustom.cpp normal i386 c++ com.apple.compilers.gcc.4_2 (2 failures)
Eric Seidel (no email)
Comment 9 2009-11-13 14:14:16 PST
Comment on attachment 43136 [details] New patch: added some closing </li>'s I'm in the process of moving the commit-queue to a new machine and it's had hiccups. Sorry. :(
WebKit Commit Bot
Comment 10 2009-11-13 16:25:43 PST
Comment on attachment 43136 [details] New patch: added some closing </li>'s Clearing flags on attachment: 43136 Committed r50978: <http://trac.webkit.org/changeset/50978>
WebKit Commit Bot
Comment 11 2009-11-13 16:25:47 PST
All reviewed patches have been landed. Closing bug.
Chris Jerdonek
Comment 12 2009-11-13 17:51:25 PST
(In reply to comment #4) > (From update of attachment 43136 [details]) > Someone should run the file through an HTML validator and fix issues they find. > > r=me on this change I did this while preparing the patch, and there was only one issue. The <style> tag specifying a style unique to this page (the style for code excerpts) occurs in the <body> rather than the <head>. This is partly a consequence of how the site is structured in source control with the <body> of each page being included, the <head> lying in another file, etc. So I didn't try addressing it in this patch. By the way, the document type for the page is HTML 4.01 strict, which doesn't really require closing <li>'s. If we want to switch to XHTML for the site, perhaps that should be made as a decision. It would be like deciding on a style guideline for the HTML in addition to the code.
Chris Jerdonek
Comment 13 2009-11-13 18:00:20 PST
(In reply to comment #5) > (From update of attachment 43130 [details]) > Be sure to mark your old patches as obsolete when uploading replacement > patches. Some tools, like bugzilla-tool post-diff or bugzilla-tool > post-commits will do this automatically for you. I wanted to give the reviewer the option of selecting either patch, given the extra information I supplied with the second patch. Is there a better way for me to do that? Also, I apologize in advance if I shouldn't be adding comments to a closed bug report. Let me know if there is a better way to handle that as well. Thanks for your help as I learn the ropes.
Darin Adler
Comment 14 2009-11-14 11:13:35 PST
(In reply to comment #12) > By the way, the document type for the page is HTML 4.01 strict, which doesn't > really require closing <li>'s. I was surprised to learn this. > If we want to switch to XHTML for the site, perhaps that should be made as a decision. Absolutely not! My desire to close an <li> element was not driven at all by a desire to use XHTML. I just have a habit of closing elements that are not self-closing. My ignorance -- sorry.
Note You need to log in before you can comment on or make changes to this bug.