Skip to content

Fix conflict on header strings.h #84

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

Closed
wants to merge 2 commits into from
Closed

Conversation

Louson
Copy link

@Louson Louson commented Aug 16, 2018

The POSIX header is needed for the functions strcasecmp and strncasecmp. But a
header strings.h in RedisModuleSDK is prefered because of the -I specified.

Considering the way the library is used in the SDK, it seems a better practice
to specify that the header strings.h is in the rmutils directory and include
RedisModuleSDK straight.

Also, there is no need to have redismodule.h in the source as it is already
present in the SDK directory.

The POSIX header is need for the functions strcasecmp and strncasecmp. But a
header strings.h in RedisModuleSDK is prefered because of the -I specified.

Considering the way the library is used in the SDK, it seems a better practice
to specify that the header strings.h is in the rmutils directory and include
RedisModuleSDK straight.

Also, there is no need to have redismodule.h in the source as it is already
present in the SDK directory.
@CLAassistant
Copy link

CLAassistant commented Aug 16, 2018

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link

Stale pull request message

@lgtm-com
Copy link

lgtm-com bot commented Mar 2, 2020

This pull request fixes 7 alerts when merging dfabfc3 into a087e7a - view on LGTM.com

fixed alerts:

  • 7 for Implicit function declaration

@gkorland gkorland requested a review from mnunberg March 3, 2020 10:15
gavrie pushed a commit that referenced this pull request Apr 7, 2020
* Updated README
* Separates Mac OS & Linux Run sections.
@gkorland gkorland closed this Apr 20, 2020
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.

3 participants