Skip to content

Conversation

@jawshooah
Copy link
Collaborator

Add an opt-in hook to run rspec before push.

It would be great to enable live terminal output during the hook run to show progress, but I'm not sure how we would go about that with the current framework.

Also, if individual tests write to stderr, then simply concatenating stdout with stderr doesn't preserve the interleaving and isn't an accurate portrayal of the actual output. Again, not sure how to fix this.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you aren't returning any line information, you can simplify this to:

[:fail, output]

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I thought that form was deprecated.

@sds
Copy link
Owner

sds commented Apr 4, 2015

Live streaming of terminal output would be quite the undertaking, especially since we want to move towards parallelized hook runs at some point. The complexity it would introduce would also be something I'd rather not introduce if we don't have to.

We aren't including any file or line information, so creating
a singleton Message array is overkill.
@jawshooah
Copy link
Collaborator Author

True, probably not worth the effort or complexity. And I guess there's really no other way to preserve the order of messages printed to stdout and stderr.

With that in mind, do you think it would be better to show only one or the other, rather than concatenating the two? E.g. if stdout is non-empty, ignore stderr.

@sds sds added the enhancement label Apr 6, 2015
@sds
Copy link
Owner

sds commented Apr 6, 2015

I think it depends on the tool. I'm not familiar with when RSpec outputs to stdout versus stderr. I think we should output whatever information is most informative in helping the developer fix the problem.

Thanks for the feature!

@sds sds closed this Apr 6, 2015
@jawshooah jawshooah deleted the pre-push/rspec branch April 6, 2015 01:07
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.

2 participants