Bug 158648

Summary: Air.js should have some documentation
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: Tools / TestsAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, lforschler, rniwa
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
the patch
none
the patch keith_miller: review+

Filip Pizlo
Reported 2016-06-10 17:19:20 PDT
Patch forthcoming.
Attachments
the patch (8.03 KB, patch)
2016-06-10 17:28 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (8.34 KB, patch)
2016-06-10 17:35 PDT, Filip Pizlo 		keith_miller: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2016-06-10 17:28:41 PDT
Created attachment 281064 [details] the patch
Filip Pizlo
Comment 2 2016-06-10 17:35:11 PDT
Created attachment 281066 [details] the patch Made minor fixes.
Keith Miller
Comment 3 2016-06-10 17:44:30 PDT
Comment on attachment 281066 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=281066&action=review r=me with some comments. > PerformanceTests/Air.js/README.md:35 > +more readable. Unlike the original C++ Air, Air.js doesn't take advantage of > +deep understanding of compilers to make the code easy to compile. I think I would phrase this as "Unlike the original C++ Air, Air.js doesn't exploit a deep understanding of compilers to make the code easy to compile." The extra prepositions make the sentence seem clunky. > PerformanceTests/Air.js/README.md:106 > +C++ Air and Air.js, and we assert that for each payload looks identical after "that for each" => each > PerformanceTests/Air.js/README.md:109 > +allcoateStack, to help catch bugs from payload initialization. We have not Nit: from => during
Filip Pizlo
Comment 4 2016-06-10 17:46:32 PDT
(In reply to comment #3) > Comment on attachment 281066 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=281066&action=review > > r=me with some comments. > > > PerformanceTests/Air.js/README.md:35 > > +more readable. Unlike the original C++ Air, Air.js doesn't take advantage of > > +deep understanding of compilers to make the code easy to compile. > > I think I would phrase this as "Unlike the original C++ Air, Air.js doesn't > exploit a deep understanding of compilers to make the code easy to compile." > The extra prepositions make the sentence seem clunky. Yeah, that's much better. > > > PerformanceTests/Air.js/README.md:106 > > +C++ Air and Air.js, and we assert that for each payload looks identical after > > "that for each" => each Fixed. > > > PerformanceTests/Air.js/README.md:109 > > +allcoateStack, to help catch bugs from payload initialization. We have not > > Nit: from => during Fixed. Thanks for the review!
Filip Pizlo
Comment 5 2016-06-10 17:47:48 PDT
Landed in http://trac.webkit.org/changeset/201957
Note You need to log in before you can comment on or make changes to this bug.