Skip to content

[Process] Renamed flushOutput() and flushErrorOutput() to clearOutput() and clearErrorOutput() #9407

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

Merged
merged 1 commit into from
Nov 9, 2013

Conversation

webmozart
Copy link
Contributor

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

I renamed flushOutput() and flushErrorOutput() to clearOutput() and clearErrorOutput() since I find the current naming unintuitive. When reading the blog post about this feature I initially thought that the output would be flushed to the console (as in ob_flush()).

Using "clear" on the other hand conforms well with our conventions.

@yguedidi
Copy link
Contributor

Same throught when reading the blog post, so 👍

@imobilis
Copy link

Nice one @bschussek! Thank you :)

@raziel057
Copy link
Contributor

+1

2 similar comments
@sstok
Copy link
Contributor

sstok commented Oct 30, 2013

👍

@noetix
Copy link

noetix commented Oct 31, 2013

👍

@staabm
Copy link
Contributor

staabm commented Nov 6, 2013

Discussed exactly that in the origin PR with fabpot
#8288 (comment)

@noetix
Copy link

noetix commented Nov 6, 2013

I was confused by this PR until I realised that I misread the blog example. I thought flush sent the buffer and then cleared it, similar to PHP's ob_flush.

Both PHP and fabpot's usage of the word is correct, to flush is to send something down a path, typically to cleanse.

So with that ambiguity and PHP precedence, wouldn't it be more concise to go with clear?

@webmozart
Copy link
Contributor Author

ping @fabpot

fabpot added a commit that referenced this pull request Nov 9, 2013
… clearOutput() and clearErrorOutput() (bschussek)

This PR was merged into the master branch.

Discussion
----------

[Process] Renamed flushOutput() and flushErrorOutput() to clearOutput() and clearErrorOutput()

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

I renamed `flushOutput()` and `flushErrorOutput()` to `clearOutput()` and `clearErrorOutput()` since I find the current naming unintuitive. When reading [the blog post about this feature](http://symfony.com/blog/new-in-symfony-2-4-flushing-stdout-and-stderr-on-a-process) I initially thought that the output would be flushed to the console (as in `ob_flush()`).

Using "clear" on the other hand conforms well with [our conventions](http://symfony.com/doc/current/contributing/code/conventions.html).

Commits
-------

1811367 [Process] Renamed flushOutput() and flushErrorOutput() to clearOutput() and clearErrorOutput()
@fabpot fabpot merged commit 1811367 into symfony:master Nov 9, 2013
@webmozart webmozart deleted the rename-flush branch January 14, 2014 16:32
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.

8 participants