8000 [Console] Add progress indicator helper by kbond · Pull Request #12119 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Console] Add progress indicator helper #12119

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.

Sign up for GitHub

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

Closed
wants to merge 8 commits into from
Closed

[Console] Add progress indicator helper #12119

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
abacef0
added progress indicator helper
kbond Oct 3, 2014
2480070
add speed trap to prevent flashing indicator
kbond Oct 9, 2015
f1144a3
cleanup comments and ProgressIndicator::setIndicatorValues error chec…
kbond Oct 13, 2015
e9c18ab
make ProgressIndicator immutable
kbond Oct 13, 2015
faab1d4
add @internal 5.3 compat flag for public setters
kbond Oct 13, 2015
01b8e1a
ensure ProgressIndicator can be used multiple times with one instance
kbond Oct 13, 2015
7bcabb4
mark ProgressIndicator::getMessage as internal
kbond Oct 13, 2015
12bf736
fix test on windows
kbond Oct 19, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
mark ProgressIndicator::getMessage as internal
  • Loading branch information
@kbond
kbond committed Oct 13, 2015
commit 7bcabb41cba5bbfe6ae643d273080a7b40ca4bf6
2 changes: 2 additions & 0 deletions src/Symfony/Component/Console/Helper/ProgressIndicator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ public function setMessage($message)
* Gets the current indicator message.
*
* @return string|null
*
* @internal for PHP 5.3 compatibility
*/
Copy link
Member

Choose a reason for hiding this comment

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

This one is also internal, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just getMessage - I will mark as such. Unless you want me to remove setMessage and just use advance to set a new message.

public function getMessage()
Copy link
Member

Choose a reason for hiding this comment

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

I would remove this accessor

Copy link
Member Author

Choose a reason for hiding this comment

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

Will this cause issues in 5.3 (https://github.com/symfony/symfony/pull/12119/files#diff-c69956b5e0c001490b2ebfa0b9e2db1aR290) or will this only be in 3.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

If we don't need 5.3 support, I should remove the other public getters as well.

Copy link
Member

Choose a reason for hiding this comment

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

If we were to merge this in 2.8 (we should act fast now), PHP 5.3.9 is still supported. If not, it's going to be merged in 3.1.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will leave the getters for now to try and get this into 2.8. If it doesn't make it, I will remove for 3.1.

Copy link
Member

Choose a reason for hiding this comment

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

if it is public only for PHP 5.3 compat, it should be marked as internal though (explaining that it is there only for compat reasons), as we do elsewhere

{
Expand Down
0