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

Outupting an error when a source is configured but no content is found #199

Merged
merged 1 commit into from Feb 6, 2015

Conversation

mikeSimonson
Copy link
Contributor

No description provided.

@simensen
Copy link
Member

simensen commented Feb 3, 2015

@mikeSimonson Thanks! I'd like to get a second set of eyes on this just because.

For a little background here, the idea is to find at least some way to warn people when a content type is configured but there are not sources found for that content type.

Longer-term, we need to find a better way to tie into logging/output so that we do not litter the codebase with echo or print statements. In the meantime I think that having some output in these cases is preferable to just being silent.

@mikeSimonson
Copy link
Contributor Author

I am also looking at making that a bit cleaner. You mentioned using the output object from the command. (outputInterface) I realised that this output is encapsulated into the consoleIo object and I looked there if I could just add a log method.
Playing around for now

@simensen
Copy link
Member

simensen commented Feb 3, 2015

@mikeSimonson Yeah, there is a subtle difference between "output" and "logging" and the ConsoleIO is almost exclusively used for "output"... any logging that should go to the screen would probably go through here as well. The abstractions I would see would be things like logError() that would wrap the incoming text w/ the appropriate tags so that it shows up as red for example. But, again, none of this is written and getting this class to the appropriate places might be difficult now. I don't think we need to worry about it for this PR but we can definitely update this code to use the logging/output system once it is in place!

@mikeSimonson
Copy link
Contributor Author

Sorry my question wasn't clear. I wanted to know if the consoleIo class was going in the right direction ?

@simensen
Copy link
Member

simensen commented Feb 3, 2015

Oh! Yes. :) That is the right direction. Though we may want our own abstraction around ConsoleIO and not use Symfony's directly. I know that Composer did that. They have their own set of interface(s) around Symfony's IO stuff.

@davedevelopment
Copy link
Contributor

👍

simensen added a commit that referenced this pull request Feb 6, 2015
Outupting an error when a source is configured but no content is found
@simensen simensen merged commit 016b840 into sculpin:master Feb 6, 2015
@simensen
Copy link
Member

simensen commented Feb 6, 2015

Thanks @mikeSimonson!

@mikeSimonson mikeSimonson deleted the source-empty branch April 27, 2015 11:52
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.

None yet

3 participants