Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MSVC: Removes unnecessary NOMINMAX macros #1454

Merged
merged 1 commit into from Aug 19, 2015
Merged

MSVC: Removes unnecessary NOMINMAX macros #1454

merged 1 commit into from Aug 19, 2015

Conversation

am11
Copy link
Contributor

@am11 am11 commented Aug 18, 2015

  • NOMINMAX macro is only required where we include <windows> header
    because it has min and max defined as macros; and we need it in
    one file: src/file.cpp.
  • Also adds .user (VS user file) to .gitignore.

@am11
MSVC: Removes unnecessary NOMINMAX macros.
c1d3f28 
* `NOMINMAX` macro is only required where we include `<windows>` header
  because it has `min` and `max` defined as macros; and we need it in
  one file: `src/file.cpp`.
* Also adds .user (VS user file) to `.gitignore`.
@am11
Copy link
Contributor Author

am11 commented Aug 18, 2015

@mgreter, separately; can we replace #ifdef DEBUG with standard C #ifndef NDEBUG postprocessor directive, so we can get libsass debug info with MSVC as well?

@saper
Copy link
Member

saper commented Aug 18, 2015

@am11 thanks for this, lgtm. So file.cpp is the only one that needs it?

@am11
Copy link
Contributor Author

am11 commented Aug 18, 2015

So file.cpp is the only one that needs it?

Yes; <windows> header is required in file.cpp for path resolution etc.

@saper
Copy link
Member

saper commented Aug 18, 2015 via email

@xzyfer
Copy link
Contributor

xzyfer commented Aug 19, 2015

Taking your word for it :)

xzyfer added a commit that referenced this pull request Aug 19, 2015
@xzyfer
Merge pull request #1454 from am11/master
5e8e268 
Removes unnecessary NOMINMAX macros
@xzyfer xzyfer merged commit 5e8e268 into sass:master Aug 19, 2015
@xzyfer xzyfer added the Build label Aug 19, 2015
@xzyfer xzyfer added this to the 3.3 milestone Aug 19, 2015
@am11
Copy link
Contributor Author

am11 commented Aug 19, 2015

Taking your word for it :)

Fingers crossed! 👌 ✌️

@saper
Copy link
Member

saper commented Sep 2, 2015

@am11 Just filed #1518 to deal with -DDEBUG

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Build OS - Windows
Projects
None yet
Milestone
3.3
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@am11 @saper @xzyfer