-
Notifications
You must be signed in to change notification settings - Fork 72
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
Hey! Spec suite! Leave them streams alone! #175
Conversation
Apparently this https://gist.github.com/adamstegman/926858 was to get RSpec to stfu. But now it calmed down again, so not necessary. And it affects other things, like swallowing print statements, causing pry to omit output, and hiding other output that is potentially legitimate. However, one nice side effect was that it caught other loudmouths in the dragnet: * ActiveRecord::Migration, apparently there is a globaly flag you set to tell it to shush. * Thor::Shell... I gave up on this one. Had tried passing in `shell` like any sensible interface, but it was blowing up pretty hard. Had tried wiping out here https://github.com/erikhuda/thor/blob/53b3fc89ce750258e19c53b1a99b32dd5941451a/lib/thor/shell.rb#L6 but it's passing around classes instead of instances, so its like you have to have layers on layers of fake classes. Tried passing it here https://github.com/erikhuda/thor/blob/53b3fc89ce750258e19c53b1a99b32dd5941451a/lib/thor/shell.rb#L53 but the indirection of who'se inheriting from whom and calling what on what and which vars got set by whatever wherever were just too much for me at this hour, so fuck it. Stub it. * CodeClimate is logging directly to stderr. You can get it to stfu with: `CodeClimate::TestReporter.configuration.logger = Logger.new('/dev/null')` Put it before the `CodeClimate::TestReporter.start` I didn't do it b/c it was doing this before (it runs before output streams were silenced) so I figured maybe someone wanted it that way on purpose. Plus, you probaly do want it logging in CI, which you'd probably do by setting an env var there, and then silencing it unless you see the var. Since I don't have access to your CI, and getting all that config wired up right tends to be fragile, I figured it'd be best to leave it. Went with a matrix theme, b/c the vindictive side of me enjoyed the idea of Thor's stupid mouth getting melted shut.
# Apparently Thor's last name is Odinson... who knew?! | ||
# http://marvel.com/universe/Thor_(Thor_Odinson) | ||
# http://www.infinitelooper.com/?v=4D7cPH7DHgA#/170;230 | ||
def tell_me_mr_odinson_what_good_is_a_phone_call_if_youre_unable_to_speak? |
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.
what a weird name!
But, this method should probably be called slience_thor
. Or named whatever the actual function is doing.
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.
Maybeh. But since the function is reveling in melting shut Thor's loud smug mouth, I think it fits. I included a video for context.
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.
still hanging on to that method name?
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.
From context, I assume you want me to change the method name, but that comment seems passive aggressive, so I'm not going to. You're just as capable of changing it as I am, you could have merged it, changed it, pushed it, a month ago, with three minutes worth of effort. I'm not going to sanitize the humour for you.
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.
Oh, apologies. I didn't mean to come off as rude. I just wasn't sure if you were being serious about the naming. Also, I thought it was poor form to push changes to other people's PRs. If that's how you feel though, I suppose...
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.
Fair enough, I probably predisposed my self to interpret it that way, due to my own emotional immaturity -- primarily a distrust/dislike of authority, which shades my view of anyone who seems to express it. That's my shit, not your fault, no need to apologize. I'll put more emphasis in my brain on remembering everyone's humanity and giving them the benefit of the doubt :)
But yeah, I like this method name. It gives me a sense of serenity. I have no problem with you changing it to better suit your coding standards, but if I have to be the one to do it, it'll feel like an imposition. So, seems best for both of us.
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.
👍 ❤️ 💯
Amendment to bellycard#175 This line shouldn't have made it in, it was a test to check if this would fix CodeClimate logging to stderr about how it's not going to do any reporting. But I meant to omit it b/c IDK how Code Climate works on Travis. I'm not even really sure this is the best way to do that. For example, an if statement around the require and start might make more sense than neutering its logging. Actually, writing that last sentence just incited me to look at the [docs](http://docs.travis-ci.com/user/ci-environment/#Environment-variables). Appears that you could just do this: ```ruby if ENV['CI'] == 'true' require "codeclimate-test-reporter" CodeClimate::TestReporter.start end ``` I'll let y'all do that, though, b/c where you report test coverage from is a different issue than the test suite swallowing its own output.
+1 |
Hey! Spec suite! Leave them streams alone!
Amendment to bellycard/napa#175 This line shouldn't have made it in, it was a test to check if this would fix CodeClimate logging to stderr about how it's not going to do any reporting. But I meant to omit it b/c IDK how Code Climate works on Travis. I'm not even really sure this is the best way to do that. For example, an if statement around the require and start might make more sense than neutering its logging. Actually, writing that last sentence just incited me to look at the [docs](http://docs.travis-ci.com/user/ci-environment/#Environment-variables). Appears that you could just do this: ```ruby if ENV['CI'] == 'true' require "codeclimate-test-reporter" CodeClimate::TestReporter.start end ``` I'll let y'all do that, though, b/c where you report test coverage from is a different issue than the test suite swallowing its own output.
Apparently this https://gist.github.com/adamstegman/926858 was to get
RSpec to stfu. But now it calmed down again, so not necessary.
And it affects other things, like swallowing print statements,
causing pry to omit output, and hiding other output that is potentially
legitimate.
However, one nice side effect was that it caught other loudmouths in the dragnet:
tell it to shush.
shell
like any sensible interface, but it was blowing up pretty hard. Had
tried wiping out here https://github.com/erikhuda/thor/blob/53b3fc89ce750258e19c53b1a99b32dd5941451a/lib/thor/shell.rb#L6
but it's passing around classes instead of instances, so its like you
have to have layers on layers of fake classes. Tried passing it here
https://github.com/erikhuda/thor/blob/53b3fc89ce750258e19c53b1a99b32dd5941451a/lib/thor/shell.rb#L53
but the indirection of who'se inheriting from whom and calling what on
what and which vars got set by whatever wherever were just too much
for me at this hour, so fuck it. Stub it.
CodeClimate::TestReporter.configuration.logger = Logger.new('/dev/null')
Put it before the
CodeClimate::TestReporter.start
I didn't do it b/cit was doing this before (it runs before output streams were silenced)
so I figured maybe someone wanted it that way on purpose. Plus, you
probably do want it logging in CI, which you'd probably do by setting an
env var there, and then silencing it unless you see the var. Since I
don't have access to your CI, and getting all that config wired up
right tends to be fragile, I figured it'd be best to leave it.
Went with a matrix theme, b/c the vindictive side of me enjoyed the idea
of Thor's stupid mouth getting melted shut.