Closed 1ma closed 10 months ago
@1ma Could you do a reroll and fix the conflicts?
Done :+1:
I've pushed the strict types in a separate commit https://github.com/swentel/nostr-php/pull/39/commits/b72efcd3ba88e8702beed02097b70e8cb9ed6477.
If it's a no-go I can drop it again, but as I said I think it's very much worth it.
Yes I like strict coding. Just tested it, works like a charm. I don't expect a lot of broken things when this is merged.
As discussed, this PR forces the project to follow the PER Coding Style 2.0 standard using the new CI pipeline.
I chose php-cs-fixer over phpcs because it can automatically format the code, not just detect violations. I chose the PER standard because it's not very opinionated compared to Symfony or PhpCsFixer's own ruleset. The actual rules applied by each ruleset can be browsed here.
As a development convenience I also introduced a
composer lint
command to easily format the code from the command line.Strict Types
We could take this opportunity to force the declaration of strict types on all classes (
'declare_strict_types' => true
). It's not strictly a backwards-compatible change (if existing users of nostr-php are not calling some methods of the library correctly), but strict types straight out prevent whole hosts of bugs.I think strict types are always worth it, but I left them out of the PR for now. I'd like other people's input on this.