Skip to content

Use / for include paths, not \ - #18

Merged
Kenny Kerr (kennykerr) merged 1 commit into
microsoft:masterfrom
LegalizeAdulthood:develop
Oct 20, 2016
Merged

Use / for include paths, not \#18
Kenny Kerr (kennykerr) merged 1 commit into
microsoft:masterfrom
LegalizeAdulthood:develop

Conversation

@LegalizeAdulthood

Copy link
Copy Markdown
Contributor

I was an MVP for Direct3D for 10 years, you might already have a CLA on file for me...

@raffaeler

Copy link
Copy Markdown

I believe it does not make sense to make pull requests on the libraries as it is all generated code.
The patch should be always applied to the generator.

@BrentRe

Copy link
Copy Markdown
Contributor

Actually this particular request is to update the Sample projects, which are not generated code.

@raffaeler

Copy link
Copy Markdown

oops, sorry

@LegalizeAdulthood

Copy link
Copy Markdown
Contributor Author

Yes, as Brent Rector (@BrentRe) pointed out, the libraries are fine, it's just the sample code that is using \ instead of /.

They also use #pragma lib, which isn't portable C++, but noone else besides Windows has a WinRT runtime anyway, so I doubt that part really matters.

@BrentRe

Copy link
Copy Markdown
Contributor

We could move the #pragma lib into the VC++ project settings to remove it. But as all these samples are Windows-specific, I'm not sure there's much benefit. The reasoning for putting in the source code was to alert developers that Windows Store applications must link with that library. Something that's easily missed when it buried in a linker command line setting.

For the record, what are you suggesting to change from back slashes to forward slashes? I'm guessing because both work and forward is more platform agnostic... I'm not pushing back but these samples are very Windows specific (as is C++/WinRT currently).

@LegalizeAdulthood

Copy link
Copy Markdown
Contributor Author

Using \ in the paths of include files only works on Windows and inculcates bad habits in C++ developers, so yes that is what this patch fixes.

@kennykerr
Kenny Kerr (kennykerr) merged commit a890b90 into microsoft:master Oct 20, 2016
@kennykerr

Copy link
Copy Markdown
Collaborator

Thanks for the contribution Richard!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants