-
Notifications
You must be signed in to change notification settings - Fork 36
Updated examples to work with Accumulo 2.0.0-alpha-1 #28
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
Conversation
milleruntime
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a comment about the examples being CLI, which reminded me we should get back to making the examples better. Maybe create follow on tickets for the classes you added CLI options.
| <groupId>commons-configuration</groupId> | ||
| <artifactId>commons-configuration</artifactId> | ||
| <version>1.6</version> | ||
| <version>1.10</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't these deps match Accumulo? Or are we able to do better here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1.10 matches what Accumulo uses
| @Parameter(names = "-c") | ||
| String clientProps = "conf/accumulo-client.properties"; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to get back to moving examples away from the CLI stuff but I guess its OK to just get a fix in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These were added for ExamplesIT. I am OK with CLI stuff if the options are visible and in the example and not abstracted away. We could remove this if we want to stop testing the examples in ExamplesIT
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could still have ExamplesIT that runs all the examples but does it programmatically instead of using the CLI.
No description provided.