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

Move various string utility functions into string_util #2094

Closed
wants to merge 4 commits into from

Conversation

sagar0
Copy link
Contributor

@sagar0 sagar0 commented Apr 5, 2017

This is an effort to club all string related utility functions into one common place, in string_util, so that it is easier for everyone to know what string processing functions are available. Right now they seem to be spread out across multiple modules, like logging and options_helper.

Test plan:

  • Ran 'make check' locally.
  • Run all remaining tests in travis and sandcastle.

@facebook-github-bot
Copy link
Contributor

@sagar0 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@sagar0 updated the pull request - view changes - changes since last import

@IslamAbdelRahman
Copy link
Contributor

Looks great! let's wait for the tests and then land.
Also If you don't mind I will go ahead and land my approved PR
#2055
I think you will need to rebase after I land it

@facebook-github-bot
Copy link
Contributor

@sagar0 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@sagar0
Copy link
Contributor Author

sagar0 commented Apr 5, 2017

Yup, I will wait for the tests; I am already closely monitoring them. I just updated the PR with minor fixes for java and lite versions.

I think you will need to rebase after I land it

That's fine. I'll rebase.

@siying
Copy link
Contributor

siying commented Apr 6, 2017

Sorry I landed #2090 so you need to resolve more conflicts now.

Summary:
This is part of an effort to club all string related utility functions
into a common place, in string_util.
logging.h should contain only contain functions related to logging.
This is part of an effort to club all string related utility functions
into a common place, in string_util.
options_helper.[h,cc] has many string utility functions that could be
useful in other places, and need to be in a more common place like
string_util.

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Blame Revision:
Exceptions are not enabled in RocksDB Lite. So ifdef out functions that
could throw exceptions.
@facebook-github-bot
Copy link
Contributor

@sagar0 updated the pull request - view changes - changes since last import

@facebook-github-bot
Copy link
Contributor

@sagar0 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

mikhail-antonov pushed a commit to mikhail-antonov/rocksdb that referenced this pull request Apr 12, 2017
Summary:
This is an effort to club all string related utility functions into one common place, in string_util, so that it is easier for everyone to know what string processing functions are available. Right now they seem to be spread out across multiple modules, like logging and options_helper.

Check the sub-commits for easier reviewing.
Closes facebook#2094

Differential Revision: D4837730

Pulled By: sagar0

fbshipit-source-id: 344278a
@sagar0 sagar0 deleted the gather-string-utils branch May 2, 2017 23:48
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