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

[Validator] Allow `ConstraintViolation::__toString()` to expose codes that are not null or emtpy strings #29375

Merged
merged 1 commit into from Dec 1, 2018

Conversation

Projects
None yet
5 participants
@phansys
Copy link
Contributor

phansys commented Nov 29, 2018

Q A
Branch 2.8
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

Allow to expose 0 or "0" validation codes.

@xabbuh xabbuh added this to the 3.4 milestone Nov 30, 2018

@@ -79,13 +79,13 @@ public function __toString()
}
$propertyPath = (string) $this->propertyPath;
$code = $this->code;
$code = trim($this->code);

This comment has been minimized.

@ro0NL

ro0NL Nov 30, 2018

Contributor

trimming is a new behavior, which IMHO is not needed.

This comment has been minimized.

@phansys

phansys Nov 30, 2018

Author Contributor

I understand your point, and I agree. My concern was originated by the absence of delimiters around the printed code, so I thought that readability could be improved trimming any undesired blankspace around the code.

This comment has been minimized.

@ro0NL

ro0NL Nov 30, 2018

Contributor

the delimiter is (code and ) :)

undesired blankspace

we don't know that

This comment has been minimized.

@phansys

phansys Nov 30, 2018

Author Contributor

I mean cases like

(code    )

I think it is not clear which space comes from the dumped message and which one comes from the actual code, but I'm pretty sure this is an edge case that we can ignore.

@ro0NL

ro0NL approved these changes Nov 30, 2018

@nicolas-grekas
Copy link
Member

nicolas-grekas left a comment

(for 3.4 as 2.8 is EOLed now)

@phansys phansys changed the base branch from 2.8 to 3.4 Nov 30, 2018

@phansys

This comment has been minimized.

Copy link
Contributor Author

phansys commented Nov 30, 2018

Updated base to target 3.4.

@nicolas-grekas nicolas-grekas force-pushed the phansys:constraint_violation_tostring branch from 1cbe5dc to 7bb0fb5 Dec 1, 2018

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Dec 1, 2018

Thank you @phansys.

@nicolas-grekas nicolas-grekas merged commit 7bb0fb5 into symfony:3.4 Dec 1, 2018

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Dec 1, 2018

bug #29375 [Validator] Allow `ConstraintViolation::__toString()` to e…
…xpose codes that are not null or emtpy strings (phansys)

This PR was squashed before being merged into the 3.4 branch (closes #29375).

Discussion
----------

[Validator] Allow `ConstraintViolation::__toString()` to expose codes that are not null or emtpy strings

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

Allow to expose `0` or `"0"` validation codes.

Commits
-------

7bb0fb5 [Validator] Allow `ConstraintViolation::__toString()` to expose codes that are not null or emtpy strings

@phansys phansys deleted the phansys:constraint_violation_tostring branch Dec 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.