Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(89)

Issue 6242045: Implemented #extension and #version directives. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 10 months ago by Alok Priyadarshi
Modified:
13 years, 10 months ago
Reviewers:
kbr1
CC:
angleproject-review_googlegroups.com
Base URL:
http://angleproject.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Implemented #extension and #version directives. Committed: https://code.google.com/p/angleproject/source/detail?r=1095

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+371 lines, -12 lines) Patch
M src/compiler/preprocessor/new/Diagnostics.h View 1 chunk +5 lines, -0 lines 0 comments Download
M src/compiler/preprocessor/new/DirectiveHandler.h View 1 chunk +7 lines, -0 lines 0 comments Download
M src/compiler/preprocessor/new/DirectiveParser.cpp View 8 chunks +112 lines, -11 lines 0 comments Download
M tests/build_tests.gyp View 2 chunks +3 lines, -1 line 0 comments Download
M tests/preprocessor_tests/MockDirectiveHandler.h View 1 chunk +8 lines, -0 lines 0 comments Download
A tests/preprocessor_tests/extension_test.cpp View 1 chunk +121 lines, -0 lines 0 comments Download
A tests/preprocessor_tests/version_test.cpp View 1 chunk +115 lines, -0 lines 1 comment Download

Messages

Total messages: 2
Alok Priyadarshi
13 years, 10 months ago (2012-05-24 05:29:48 UTC) #1
kbr1
13 years, 10 months ago (2012-05-24 16:20:20 UTC) #2
LGTM

I agree with your comment on the last code review that the tests should be
refactored to introduce a base class holding the Diagnostics, DirectiveHandler
and other objects.

https://codereview.appspot.com/6242045/diff/1/tests/preprocessor_tests/versio...
File tests/preprocessor_tests/version_test.cpp (right):

https://codereview.appspot.com/6242045/diff/1/tests/preprocessor_tests/versio...
tests/preprocessor_tests/version_test.cpp:38: // No handlePragma calls.
Stale comment?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b