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

system.h: remove all CURL_SIZEOF_* defines #1767

Closed
wants to merge 5 commits into from
Closed

system.h: remove all CURL_SIZEOF_* defines #1767

wants to merge 5 commits into from

Conversation

bagder
Copy link
Member

@bagder bagder commented Aug 11, 2017

... as they're not used externally and internally we check for the sizes
already in configure etc.

@bagder bagder force-pushed the bagder/system-h-remove-sizes branch from 8be8a01 to 4dfab82 Compare August 12, 2017 13:57
@bagder bagder added the feature-window A merge of this requires an open feature window label Aug 12, 2017
@jay
Copy link
Member

jay commented Aug 13, 2017

My opinion is CURL_SIZEOF_ for curl types is appropriate. Is there some reason not to have them, and couldn't this be addressed by just removing CURL_SIZEOF_LONG?

@bagder
Copy link
Member Author

bagder commented Aug 13, 2017

I want to remove the SIZEOF defines from there because:

  1. They're not necessary
  2. They're not used by any public header/functionality
  3. They make the system.h larger than it has to be, both in total and per system

@bagder bagder force-pushed the bagder/system-h-remove-sizes branch from 4dfab82 to c486957 Compare August 15, 2017 16:54
@bagder
Copy link
Member Author

bagder commented Aug 15, 2017

Rebased and fixed merge conflicts

@bagder bagder removed the feature-window A merge of this requires an open feature window label Aug 15, 2017
@bagder bagder force-pushed the bagder/system-h-remove-sizes branch from a5c139b to 6ccbdd1 Compare August 16, 2017 05:59
@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 75.08% when pulling 6ccbdd1 on bagder/system-h-remove-sizes into 75536e5 on master.

@curl curl deleted a comment from coveralls Aug 16, 2017
@curl curl deleted a comment from coveralls Aug 16, 2017
@curl curl deleted a comment from coveralls Aug 16, 2017
@curl curl deleted a comment from coveralls Aug 16, 2017
@curl curl deleted a comment from coveralls Aug 16, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 75.062% when pulling 4731802 on bagder/system-h-remove-sizes into 75536e5 on master.

@bagder bagder closed this in 80d9e35 Aug 17, 2017
@bagder bagder deleted the bagder/system-h-remove-sizes branch August 17, 2017 08:27
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bagder @jay @coveralls