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

[PostgreNoSQL] Added NoSQL support for PostgreSQL. #1242

Merged
merged 1 commit into from
Apr 26, 2019
Merged

[PostgreNoSQL] Added NoSQL support for PostgreSQL. #1242

merged 1 commit into from
Apr 26, 2019

Conversation

leschekhomann
Copy link
Contributor

@busbey: I hope to have addressed all suggested adaptions.

  • Removed package information from testing.
  • Added caching for the SQL Statements by using the Statement Class.
  • Added additional information about code changes in the license part.
  • Simplified test code (removed creation and deletion of test database, only use travis).

@leschekhomann
Copy link
Contributor Author

@busbey: Any news here?

@busbey
Copy link
Collaborator

busbey commented Nov 21, 2018

Thanks for the reminder. I was originally going to try to close out a release before merging this, but that didn't happen. I'll try to get a review done soon.

@leschekhomann
Copy link
Contributor Author

@busbey: Thanks for the quick response and your time to review the pull request.

@leschekhomann
Copy link
Contributor Author

One additional information: Maybe it is useful to remove the padded and createFieldSet methods from the PostgreNoSQLDBClientTest as they are also implemented in other YCSB bindings. Somewhere in the core?

@leschekhomann
Copy link
Contributor Author

Hi @busbey. Any news here? Best regards

@busbey
Copy link
Collaborator

busbey commented Feb 14, 2019

I thought I merged this already. Sorry!

I'll take a look during my block of open source time tomorrow.

@leschekhomann
Copy link
Contributor Author

Hi @busbey. No problem. Thanks for checking the pull request. Please check my comments above and in the code. There are some things I am not sure about, for instance, reusing code of other bindings of the YCSB project.

@leschekhomann
Copy link
Contributor Author

As you can see in the license notes of the sources some of the code is based on the jdbc binding.

@leschekhomann
Copy link
Contributor Author

Hi @busbey. Any news?

@busbey busbey merged commit 29f254d into brianfrankcooper:master Apr 26, 2019
@leschekhomann
Copy link
Contributor Author

@busbey: Thanks for accepting the pull request. How about my comments regarding the license and source code adoptions? Everything fine so far?

@busbey
Copy link
Collaborator

busbey commented Apr 26, 2019

Yeah that's fine. We can always try to deduplicate later.

busbey added a commit to busbey/YCSB that referenced this pull request Jun 4, 2019
@busbey
[core] add missing license headers.
264734d 
* brianfrankcooper#1249 missed headers when moving things out of Client.java
* brianfrankcooper#1242 missed header on a properties file is added
* brianfrankcooper#1286 missed header on the new pom file
@busbey busbey mentioned this pull request Jun 4, 2019
Merged
busbey added a commit that referenced this pull request Jun 4, 2019
@busbey
[core] add missing license headers. (#1313)
bafc4de 
* #1249 missed headers when moving things out of Client.java
* #1242 missed header on a properties file is added
* #1286 missed header on the new pom file
@busbey busbey mentioned this pull request Jun 4, 2019
Closed
@busbey busbey mentioned this pull request Sep 21, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@busbey busbey busbey approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@leschekhomann @busbey