Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Console] Add ability to regress the ProgressBar#19824
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
jameshalsall commentedSep 2, 2016
The build failure in AppVeyor does not look related to these changes. |
| * | ||
| * @param int $step Number of steps to regress | ||
| */ | ||
| publicfunctionregress($step =1) |
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.
I think we don't need a new method. What about just allowing negative numbers for advance?
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.
We could do that, but that sounds like an oxymoron callingadvance() to actually step backwards, addingregress() at least keeps the API consistent
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.
But it would keep it consistent with the progress property too, although I'm fine with both, I don't see anything shocking about using negative here.
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.
I prefer a clean API with simple, straight-forward methods, myself.
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.
Then, we need an exception inadvance when used with a negative number and another one inregress when used with a negative number as well.
nicolas-grekasSep 14, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
An exception would be a DX issue, forcing one to call the correct method with boilerplate just for the sign. I agree with the first comment, we need only progress.
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.
This has now been updated so there is noregress() method.
ceecd00 to42971bbComparefabpot commentedSep 14, 2016
Thank you@jameshalsall. |
…shalsall)This PR was merged into the 3.2-dev branch.Discussion----------[Console] Add ability to regress the ProgressBar| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#15227| License | MIT| Doc PR |https://github.com/symfony/symfony-docs/pull/6949/filesCommits-------42971bb [Console] Add ability to regress the ProgressBar
…halsall)This PR was squashed before being merged into the master branch (closes#6949).Discussion----------Update ProgressBar docs with regress informationDoc updates for changes introduced insymfony/symfony#19824Commits-------d3346d8 Update ProgressBar docs with regress information
Uh oh!
There was an error while loading.Please reload this page.