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

[QA] Add PHP-CS-Fixer #87

Merged
merged 2 commits into from
Oct 15, 2020
Merged

[QA] Add PHP-CS-Fixer #87

merged 2 commits into from
Oct 15, 2020

Conversation

veewee
Copy link
Collaborator

@veewee veewee commented Oct 15, 2020

Automatically check some CS rules as described in #30

FYI : the config and the fixes are in 2 separate commits. I can change those independently if you wish to change a setting.

@veewee veewee added Status: Review Needed The issue has a PR attached to it which needs to be reviewed. Type: Enhancement Most issues will probably ask for additions or changes. labels Oct 15, 2020
@veewee veewee added this to the v1.0.0 milestone Oct 15, 2020
@veewee veewee requested a review from azjezz October 15, 2020 08:20
@veewee
Copy link
Collaborator Author

veewee commented Oct 15, 2020

The latest psalm version gives this issue:

ERROR: RedundantCondition - src/Psl/Str/convert_encoding.php:27:5 - Type string for $result is never false (see https://psalm.dev/122)
    Psl\invariant(false !== $result, 'Unable to convert $string from $from_encoding to $to_encoding.');


ERROR: RedundantCondition - src/Psl/Str/convert_encoding.php:27:19 - string can never contain false (see https://psalm.dev/122)
    Psl\invariant(false !== $result, 'Unable to convert $string from $from_encoding to $to_encoding.');


Haven't touched the file. Is it safe to remove the invariant check? Or shall I add an ignore for that rule?
Looks like it always returns a string: https://www.php.net/manual/en/function.mb-convert-encoding.php

@coveralls
Copy link

coveralls commented Oct 15, 2020

Pull Request Test Coverage Report for Build 645

  • 24 of 24 (100.0%) changed or added relevant lines in 14 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 642: 0.0%
Covered Lines: 1748
Relevant Lines: 1748

💛 - Coveralls

@azjezz
Copy link
Owner

azjezz commented Oct 15, 2020

okay; you can go ahead and remove that check.

@azjezz azjezz merged commit edb2c8c into azjezz:develop Oct 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Review Needed The issue has a PR attached to it which needs to be reviewed. Type: Enhancement Most issues will probably ask for additions or changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants