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

Cleanup type declarations to be symmetric with other function definitions. #207

Merged
merged 2 commits into from
May 15, 2017
Merged

Cleanup type declarations to be symmetric with other function definitions. #207

merged 2 commits into from
May 15, 2017

Conversation

eile
Copy link
Contributor

@eile eile commented May 10, 2017

No description provided.

@eile
Copy link
Contributor Author

eile commented May 10, 2017

6bdb3f9 is in #206. Will rebase once merged.

rdumusc
rdumusc approved these changes May 10, 2017
View reviewed changes
Copy link

@rdumusc rdumusc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we had discussed this and decided to keep the 4 function typedefs in server.h because that's the only place where they apply. There are no other http types and no benefits in separating the typedefs from the server IMO. But if you insist ;-)

zeroeq/http/types.h

#pragma once

#include <zeroeq/types.h>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is used, at least for some basic types like uint128_t

@eile
Copy link
Contributor Author

eile commented May 10, 2017

Yes, we had discussed:

  • the function typedefs for subscriber did not follow the same pattern
  • I feel that one file, one type should be a rule with the exceptions of typedefs in types.h

Imo the only alternative would be making this sub-types of the class, which is done in other places.

@eile eile force-pushed the cleanups branch 2 times, most recently from c7321c1 to e659a76 Compare May 12, 2017 11:31
@eile
Copy link
Contributor Author

eile commented May 12, 2017

Ping

@eile eile merged commit bff3a00 into HBPVIS:master May 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rdumusc rdumusc rdumusc approved these changes

Assignees

@hernando hernando

@rdumusc rdumusc

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eile @rdumusc @hernando