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

Added some reassuring output. #39

Merged
merged 1 commit into from
Aug 8, 2012
Merged

Added some reassuring output. #39

merged 1 commit into from
Aug 8, 2012

Conversation

evanmcc
Copy link
Contributor

@evanmcc evanmcc commented Jun 7, 2012

Just a few lines so that the runner of the command knows that riaknostic
is running and that it exited without error.

This fixes #32, rather.

@bsparrow435
Copy link
Contributor

This looks good to me, anyone else have input?

@ghost
Copy link

ghost commented Jun 24, 2012

Another thing to note is that we tend to standardize on creating a new branch on the basho/riaknostic repo, and submitting a pull request from there, rather than forking our own repo and submitting a pull request.

It doesn't hurt anything to do it this way, but that's just the convention.

@evanmcc
Copy link
Contributor Author

evanmcc commented Jun 25, 2012

Yeah, I had another issue with this in my rebar change. It looks like a line or two has dropped out of my .emacs file over the years. I'll get that stuff fixed.

As per the convention, I have two or three (possibly bikeshed-y) concerns there:

  • making new branches in the repo ends up littering the repo with branches that have to be manually reaped.
  • more importantly, community members cannot do this, which leads to two different processes for different contributors
  • also I kind of prefer the 'master is latest stable' strategy (this one is definitely bikesheddy)

@ghost
Copy link

ghost commented Jun 27, 2012

I wouldn't be concerned that a check would crash riaknostic. That's what we need eunit tests for, to make sure that it doesn't occur.

If a check does crash riaknostic and a user sees it, they should just report that and we get a fix out pronto.

Check out riaknostic_check:print, since that's where it actually takes the lager messages and outputs them.

@evanmcc
Copy link
Contributor Author

evanmcc commented Jun 27, 2012

Taking yet another look at the code I am going to remove this part.
It isn't actually the important part of the patch, and the
ordering/sorting issues complicate the output too much to make it
worth the effort.

On Wed, Jun 27, 2012 at 11:00 AM, Tryn Mirell
reply@reply.github.com
wrote:

I wouldn't be concerned that a check would crash riaknostic. That's what we need eunit tests for, to make sure that it doesn't occur.

If a check does crash riaknostic and a user sees it, they should just report that and we get a fix out pronto.

Check out riaknostic_check:print, since that's where it actually takes the lager messages and outputs them.


Reply to this email directly or view it on GitHub:
#39 (comment)

Just a few lines so that the runner of the command knows that riaknostic
is running and that it exited without error.
evanmcc added a commit that referenced this pull request Aug 8, 2012
@evanmcc evanmcc merged commit 3a5ed3a into basho:master Aug 8, 2012
@evanmcc evanmcc mentioned this pull request Aug 8, 2012
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.

2 participants