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

[client] Added basic command line argument error messages to Client.java #408

Merged
merged 1 commit into from
Feb 15, 2016

Conversation

petergaultney
Copy link
Contributor

This stems from my earlier adventures (see #407) trying to get a basic MySQL benchmark to run over JDBC, and improperly supplying the classpath but having no way to know what was upsetting the Java command line argument parser.

also "fixed" some indentation, because the alternative would have been to submit poorly indented code. Emacs is great for keeping indentation consistent, but if you're trying to match a file that isn't consistent, you're pretty much screwed. :)

@busbey
Copy link
Collaborator

busbey commented Sep 16, 2015

please write the messages to stderr.

nit, but if you're sufficiently familiar with git, could you update the commit message to start with "[client]"?

@busbey
Copy link
Collaborator

busbey commented Sep 16, 2015

wait, we exit with status 0 (aka "Success") in all these cases? That sounds wrong to me. anyone else?

@cmatser
Copy link
Collaborator

cmatser commented Sep 16, 2015

Exit status 0 in all cases is wrong, but I don't think you're asking @petergaultney to fix that. Right? :)

@petergaultney
Copy link
Contributor Author

I completely agree that it's wrong, but didn't feel comfortable making any decisions about whether to go with -1 or a series of different negative integers.

I always need to learn more git, so I can certainly make the commit message start with [client] once I figure it out.

While I'm at it, shall I just change the errors I'm touching to -1 and let someone else decide whether to make it more granular than that?

@busbey
Copy link
Collaborator

busbey commented Sep 17, 2015

I just wanted to make sure I wasn't missing something obvious before introducing an incompatible change.

Peter, I'm fine with you leaving the exit codes as are. If you're up for updating all of them to be -1 (or really any other non-zero number), that'd be great.

If this commit is the last one on the branch, you can edit the message easily by using git commit --amend.

While you're updating the git commit, could you break this into two patches? First one that fixes the formatting and then one with the change of functionality? (Let me know if you'd like help in how to break a commit into two.)

@busbey
Copy link
Collaborator

busbey commented Dec 31, 2015

Bump. I'd like to get this in soon.

@petergaultney think you'll have time to revisit this in the new year?

@busbey
Copy link
Collaborator

busbey commented Jan 30, 2016

@petergaultney, could you ammend the commit message? We'll take care of the other nits in a follow-on PR

@risdenk
Copy link
Collaborator

risdenk commented Feb 15, 2016

@busbey - Any thoughts about getting this into 0.7.0? #624

@busbey
Copy link
Collaborator

busbey commented Feb 15, 2016

yeah, sure.

@busbey busbey changed the title Added basic command line argument error messages to Client.java [client] Added basic command line argument error messages to Client.java Feb 15, 2016
busbey added a commit that referenced this pull request Feb 15, 2016
[client] Added basic command line argument error messages to Client.java
@busbey busbey merged commit 3b3688e into brianfrankcooper:master Feb 15, 2016
jaricftw pushed a commit to jaricftw/YCSB that referenced this pull request Jul 19, 2016
[client] Added basic command line argument error messages to Client.java
jaricftw pushed a commit to jaricftw/YCSB that referenced this pull request Jul 19, 2016
[client] Added basic command line argument error messages to Client.java
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.

4 participants