Suggestion for keeping an consistent, automatic code style (with php-cs-fixer) #92
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pscheit
commented
Jun 25, 2025
Comment on lines
72
to
+74
| $mismatchDescription->appendText('element ')->appendValue($k) | ||
| ->appendText(' was ')->appendValue($actual[$k]); | ||
| ->appendText(' was ')->appendValue($actual[$k]) | ||
| ; |
There was a problem hiding this comment.
I would probably write it more like:
$mismatchDescription
->appendText('element ')
->appendValue($k)
->appendText(' was ')
->appendValue($actual[$k])
;
pscheit
commented
Jun 25, 2025
Comment on lines
57
to
+60
| $description | ||
| ->appendText('array with key ') | ||
| ->appendDescriptionOf($this->_keyMatcher) | ||
| ; | ||
| ->appendText('array with key ') | ||
| ->appendDescriptionOf($this->_keyMatcher) | ||
| ; |
There was a problem hiding this comment.
this looks more concise to me!
pscheit
commented
Jun 25, 2025
Comment on lines
41
to
+43
| $mismatchDescription->appendText('array was ') | ||
| ->appendText('[') | ||
| ; | ||
| ->appendText('[') | ||
| ; |
There was a problem hiding this comment.
$mismatchDescription
->appendText('array was ')
->appendText('[')
;
pscheit
commented
Jun 25, 2025
| * @param mixed $item the object against which the matcher is evaluated. | ||
| * | ||
| * @return boolean <code>true</code> if <var>$item</var> matches, | ||
| * @return bool <code>true</code> if <var>$item</var> matches, |
There was a problem hiding this comment.
up for discussion of course, i like it more if its concise (but dont care if its boolean or bool)
pscheit
commented
Jun 25, 2025
|
|
||
| public function testMatchesArrayContainingKey() | ||
| { | ||
| $array = array('a'=>1, 'b'=>2, 'c'=>3); |
There was a problem hiding this comment.
thought about this a lot, but actually Hamcrest itself describes array's with => (with whitespace around the operator) and this is PSR- standard, so would change that
pscheit
commented
Jun 25, 2025
| { | ||
| assertThat((float) 5.2, floatValue()); | ||
| assertThat((double) 5.3, doubleValue()); | ||
| assertThat((float) 5.3, doubleValue()); |
There was a problem hiding this comment.
There is no difference in PHP. float, double or real are the same datatype.
At the C level, everything is stored as a double.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed a few comments about Codestyle in PRs.
To keep maintainer and contributors happy, I'd suggest to automate the codestyle. That way everyone can concentrate on the code(quality) and not on the codestyle.
Of course a code-formatter does not everything 1000% the way one wants. It's just not customizable entirely.
However this leads to a more consistent codestyle (thats why PSR-x was born) throughout different projects - which I think is a good thing.
Used:
https://cs.symfony.com/doc/usage.html
Cause I feel like this has become a defactor-standard.
Based it on PSR-12, but a lot needs customization.
Non "standard" are in this repo:
Anyway, this is just a suggestion, happy to adjust whatever you want (and we can).
Would probably add this to the pipeline, because otherwise no one runs this, and WHEN someone runs it and wants to contribute everything would be outdated.
Of course we can disable the method chaining rules and leave it in "free"-mode as is right now.
Hope you like it!