Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1036)

Issue 1078513002: Include windows.h instead of the MFC header afxres.h (Closed)

Created:
5 years, 8 months ago by brucedawson
Modified:
5 years, 8 months ago
Reviewers:
Lei Zhang, Tom Sepez
CC:
pdfium-reviews_googlegroups.com, Tom Sepez
Base URL:
https://pdfium.googlesource.com/pdfium.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Include windows.h instead of the MFC header afxres.h VS 2015 RC ships without afxres.h, so fpdfsdkdll.rc fails to compile. afxres.h is really intended for MFC apps so depending on it is a bad idea anyway, so I changed both references to windows.h. See http://stackoverflow.com/questions/1575559 for some other perspective on this. R=tsepez@chromium.org BUG=440500 Committed: https://pdfium.googlesource.com/pdfium/+/70476c21026f533ffb916b6a7ce387639f34e696

Patch Set 1 #

Patch Set 2 : Updating .gitignore file. #

Total comments: 1

Patch Set 3 : Keep .gitignore alphabetized. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -3 lines) Patch
M .gitignore View 1 2 2 chunks +2 lines, -1 line 0 comments Download
M fpdfsdk/src/fpdfsdkdll.rc View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 8 (1 generated)
brucedawson
After the const-comparator issue (see email) is fixed this is the last-known VS 2015 issue ...
5 years, 8 months ago (2015-04-08 23:42:13 UTC) #1
Tom Sepez
Rubberstamp LGTM, otherwise maybe thestig@ has the windows expertise to actually review this.
5 years, 8 months ago (2015-04-09 00:12:45 UTC) #3
brucedawson
I also updated the .gitignore file. This isn't strictly necessary but it stops git from ...
5 years, 8 months ago (2015-04-09 00:23:00 UTC) #4
Tom Sepez
LGTM after fixing nit. https://codereview.chromium.org/1078513002/diff/20001/.gitignore File .gitignore (right): https://codereview.chromium.org/1078513002/diff/20001/.gitignore#newcode24 .gitignore:24: *.aps nit: alphabetize.
5 years, 8 months ago (2015-04-09 00:27:18 UTC) #5
brucedawson
Alphabetization fixed. Any thoughts thestig@ on the .rc change?
5 years, 8 months ago (2015-04-09 00:51:35 UTC) #6
Lei Zhang
If VS is putting .aps files in the source tree somehow, then sure.
5 years, 8 months ago (2015-04-09 19:31:00 UTC) #7
brucedawson
5 years, 8 months ago (2015-04-09 20:44:59 UTC) #8
Message was sent while issue was closed.
Committed patchset #3 (id:40001) manually as
70476c21026f533ffb916b6a7ce387639f34e696 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698