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.

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

Include <cstdint> unconditionally #1835

Merged
merged 1 commit into from
Jan 12, 2016
Merged

Include <cstdint> unconditionally #1835

merged 1 commit into from
Jan 12, 2016

Conversation

saper
Copy link
Member

@saper saper commented Jan 7, 2016

A follow up to #1721

We need <cstdint> a.k.a <stdint.h> to access
width-based integer types like uint_fast8_t
@saper saper changed the title Include <inttypes.h> unconditionally Include <cstdint> unconditionally Jan 7, 2016
@saper
Copy link
Member Author

saper commented Jan 8, 2016

Not sure what PR ready means ... but hey, green looks so much better!

@mgreter
Copy link
Contributor

mgreter commented Jan 9, 2016

It's mostly just usefull for issues, but I tend to flag PRs that I feel can be "blindly" merged if needed. Just a minor nitpick, a comment in the source for such conditionals would be usefull. Other than that 👍

@xzyfer
Copy link
Contributor

xzyfer commented Jan 9, 2016

I'm all for this. We've talked bout VS2013 before. The only reason we haven't addressed it yet is that we don't have a plan for testing it in parallel with VS2015.

#1719 (comment)

@xzyfer xzyfer added this to the 3.3.3 milestone Jan 9, 2016
@saper
Copy link
Member Author

saper commented Jan 9, 2016

This change removes VS version conditional - we should always include this header because we use width int types. I tested it with VS2013 and VS2015 was tested by appveyor.

We should be able to run tests for VS2015 as an additional environment if we want.

@xzyfer
Copy link
Contributor

xzyfer commented Jan 12, 2016

Rebuilding. If CI is still happy 🚢

xzyfer added a commit that referenced this pull request Jan 12, 2016
Include <cstdint> unconditionally
@xzyfer xzyfer merged commit d8482ad into sass:master Jan 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants