Skip to content

Ensure synthetic function names conform to naming requirements #1076

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

Merged
merged 1 commit into from
Jun 1, 2016

Conversation

danmactough
Copy link
Contributor

Pull Request check-list

Please make sure to review and check all of these items:

  • Does npm test pass with this change (including linting)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Description of change

The "restore-asking" function name is not valid and was causing co-redis (by way of its usage of thenify) to throw because thenify uses the function name to rewrite async functions with promises.

This PR will change the name of the "restore-asking" function to "restore_asking", which is valid.

This sanitation is a bit stricter than necessary, since it also sanitizes valid unicode characters, but it covers this module's potential use cases just fine.

The "restore-asking" function name is not valid and was causing co-redis (by way of its usage of thenify) to throw because thenify uses the function name to rewrite async functions with promises.

This PR will change the name of the "restore-asking" function to "restore_asking", which is valid.

This sanitation is a bit stricter than necessary, since it also sanitizes valid unicode characters, but it covers this module's potential use cases just fine.
@BridgeAR BridgeAR merged commit 1487760 into redis:master Jun 1, 2016
@BridgeAR
Copy link
Contributor

BridgeAR commented Jun 1, 2016

@danmactough Thanks a lot!

@danmactough
Copy link
Contributor Author

my pleasure 😸 @BridgeAR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants