Skip to content

Update console.rst: reversing constants (logical purpose) #10787

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

Closed
wants to merge 2 commits into from
Closed

Update console.rst: reversing constants (logical purpose) #10787

wants to merge 2 commits into from

Conversation

ajie62
Copy link
Contributor

@ajie62 ajie62 commented Dec 17, 2018

For logical purpose, I added a ! right before $this->requirePassword in the configure() method, otherwise the requirePassword argument would be set as REQUIRED instead of OPTIONAL.

For logical purpose, I added a `!` right before `$this->requirePassword` in the `configure()` method, otherwise the requirePassword argument would be set as REQUIRED instead of OPTIONAL.
@ajie62 ajie62 changed the title Update console.rst Update console.rst: missing "!" for logical purpose Dec 17, 2018
@apfelbox
Copy link
Contributor

It would increase the readability, if we just reversed the two constants, don't you think?

@ajie62
Copy link
Contributor Author

ajie62 commented Dec 17, 2018

Yes, it would! Anyway, I just wanted to point out the logical issue here. Reversing the two constants would do the job! :)

Removed the "!" and just reversed the constants as suggested by Jannik Zschiesche
@ajie62 ajie62 changed the title Update console.rst: missing "!" for logical purpose Update console.rst: reversing constants (logical purpose) Dec 17, 2018
@javiereguiluz javiereguiluz added this to the 3.4 milestone Dec 17, 2018
@javiereguiluz
Copy link
Member

@ajie62 nice first contribution to Symfony Docs! Thanks and congrats on it. We merged it in 3.4 (the oldest maintained branch that contains the error) and we'll merge in the other branches automatically later.

javiereguiluz added a commit that referenced this pull request Dec 17, 2018
…) (ajie62)

This PR was submitted for the master branch but it was squashed and merged into the 3.4 branch instead (closes #10787).

Discussion
----------

Update console.rst: reversing constants (logical purpose)

For logical purpose, I added a `!` right before `$this->requirePassword` in the `configure()` method, otherwise the requirePassword argument would be set as REQUIRED instead of OPTIONAL.

<!--

If your pull request fixes a BUG, use the oldest maintained branch that contains
the bug (see https://symfony.com/roadmap for the list of maintained branches).

If your pull request documents a NEW FEATURE, use the same Symfony branch where
the feature was introduced (and `master` for features of unreleased versions).

-->

Commits
-------

9298445 Update console.rst: reversing constants (logical purpose)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants