Skip to content
This repository has been archived by the owner on May 30, 2023. It is now read-only.

remove hardcoded "PhantomJS is launching GhostDriver..." message #12681

Closed
wants to merge 1 commit into from

Conversation

paulmillar
Copy link
Contributor

Apart from being undesirable that debugging messages are always
written to stdout, the information is redundant as a similar message
is logged at debug level.

Apart from being undesirable that debugging messages are always
written to stdout, the information is redundant as a similar message
is logged at debug level.
@ariya
Copy link
Owner

ariya commented Dec 2, 2014

In release mode, this is to only way to inform the user than Ghost Driver is being launched. It is not debugging, it's informative. Or am I missing something here? /cc @detro

@paulmillar
Copy link
Contributor Author

As you might guess, I want to run PhantomJS with GhostDriver to run some functional tests. When everything is working correctly, the message "PhantomJS is launching..." is redundant. I have no interest in seeing it -- indeed, I actively don't want to see it. It's noise: printed each time PhantomJS is started.

Of course, there are times when I do want to know that GhostDriver is starting. This is when I am debugging a problem and want to know why something isn't working the way I expect it to. In this case, I can enable debug-level logging and see an equivalent message. So this particular printf serves no strong purpose.

I'm no alone in wanting GhostDriver to operate more quietly; e.g., see: https://github.com/detro/ghostdriver/issues/338

This post describes how to silence most of those annoying messages, except the hard-coded one. This patch is to silence that final message.

@JamesMGreene
Copy link
Collaborator

Makes sense to me... unless it is a standard-ish WebDriver thing that I'm unaware of.

Perhaps we should consider a verbose/quiet/logLevel config flag for runtime verbosity instead, though?

@paulmillar
Copy link
Contributor Author

I've not see other WebDriver implementations have it hard-coded that they log they started, only PhantomJS.

Note that the line immediately before logs (almost) the same information, but does so using the existing logging framework. Therefore, the line this patch removes is completely redundant: if you want to see it, switch on debug output.

There's a secondary question: how do you switch on debug output. The code is a little convoluted, so it wasn't clear to me.

@ariya
Copy link
Owner

ariya commented Jan 9, 2015

I think this is completely reasonable. I will land the patch, thank you!

ariya pushed a commit that referenced this pull request Jan 9, 2015
Apart from being undesirable that debugging messages are always
written to stdout, the information is redundant as a similar message
is logged at debug level.

#12681
@ariya ariya closed this Jan 9, 2015
kjelloe pushed a commit to kjelloe/phantomjs that referenced this pull request Feb 2, 2015
Apart from being undesirable that debugging messages are always
written to stdout, the information is redundant as a similar message
is logged at debug level.

ariya#12681
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants