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

Friendly key warnings #6500

Merged
merged 1 commit into from Apr 16, 2016
Merged

Friendly key warnings #6500

merged 1 commit into from Apr 16, 2016

Conversation

hkal
Copy link
Contributor

@hkal hkal commented Apr 13, 2016

Resolves #6475

Here's a first pass at this. I'm not entirely sold on the naming. Perhaps something like wrap() and unwrap() might be a more appropriate name for these helper functions?

cc @jimfb

* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule keyEscapeUtils
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's capitalize the 'k' to become KeyEscapeUtils. The reason being that we capitalize classes (an object full of methods is basically a class) and reserve lowercase for function names.

@jimfb
Copy link
Contributor

jimfb commented Apr 13, 2016

Overall, this looks good to me. A couple of comments to fix. Also, if you can squash your commits (git rebase -i, git push -f). Other than that, I think this is good to go.

@hkal hkal force-pushed the friendly-key-warnings branch 2 times, most recently from 332b349 to f9131dc Compare April 15, 2016 01:51
@facebook-github-bot
Copy link

@hkal updated the pull request.

- Abstract escaping
- Provide human readible same key warnings
@jimfb jimfb merged commit dc6fc8c into facebook:master Apr 16, 2016
@jimfb
Copy link
Contributor

jimfb commented Apr 16, 2016

Thanks @hkal

@hkal hkal deleted the friendly-key-warnings branch April 16, 2016 06:58
@zpao zpao modified the milestones: 15.0.x, 15.0.2 Apr 22, 2016
zpao pushed a commit that referenced this pull request Apr 28, 2016
- Abstract escaping
- Provide human readible same key warnings
(cherry picked from commit dc6fc8c)
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.

None yet

4 participants