big refactor #17
big refactor #17
Conversation
|
@prewk sorry for the massive PR. |
Lctrs
force-pushed
the
v4
branch
2 times, most recently
from
cffebf0
to
be1fbcb
Compare
|
Awesome stuff, I'll look at it as soon as I can. |
|
Everything is great and you're probably right about
I'll bump (1) as a minor and (2) as a major. Thanks for your hard work! |
|
There are other breaking changes than the removal of pass properties, making the classes final is one of them (forgot to add that in the changelog). |
|
Alright, are you able to split that up as well? If not, at the very least - fix #16 in one commit and the rest in another, would that be okay? |
|
Will see what I can do. |
* dropping support for `PHP < 8.1.0` * simplify phpdoc types, making `Option` the only source of truth for most methods * added some static analysis assert for `Option#isSome()` and `Option#isNone()` (closes prewk#16) * added `Option#inspect()` and `Option#filter()` from their Rust counterparts * migrating from phpspec to PHPUnit * added a Makefile to help during development * added a coding standard via `easy-coding-standard` * added mutation testing coverage via `infection` * added some static analysis tests * modernize github actions setup * added dependabot config * upgrading all dependencies
|
@prewk I removed:
There should not be any BC left. |
|
Awesome! I'd love for the BC stuff as well, but as its own commit, then I can tag the commit appropriately afterwards 👍 |
|
Will do once this got merged. |
|
Side note: we could integrate Roave/BackwardCompatibilityCheck to check if no BC has been introduced. |
Sure, if you prefer. I'll tag afterwards anyway so it doesn't really matter to me. (I'll need to look through the changes a bit deeper before merging)
Looks like a fun/great tool, not sure if this extremely slowly moving lib needs even more CI tooling around it tho :) Too much to maintain. |
It's just to be able to have newer CI setup with upgraded deps :) |
|
I see, that makes sense 👍 |
|
The PHP 8.1 is nice but also BC 🤔 Maybe too hard to split out at this stage? Maybe I should backpedal here and just do all your stuff with one big major bump instead.. |
|
Would say that dropping support for some PHP versions is not a BC for me, but some method's signatures do have changed. Even if they're still aligned with what was said in phpdoc, this can still be considered as BC. |
|
If I'm using this lib on a PHP version with dropped support, doing a minor update would be a BC I guess? Anyway, sorry for making you do unnecessary work. Just throw it all in as your original intent was, I'll do one release with a major bump instead. (Still reviewing) |
| * @covers \Prewk\Option | ||
| * @uses \Prewk\Option\None | ||
| * @uses \Prewk\Option\Some |
There was a problem hiding this comment.
question: What are these for? Coveralls?
There was a problem hiding this comment.
Yes, and for infection to know what to do.
tl;dr @covers is for code that should be included in coverage reports, and modified by infection. @uses is for code used by the tests, but to not be included in coverage reports, or modified by infection.
| */ | ||
| function baz(None $none): void | ||
| { | ||
| } |
There was a problem hiding this comment.
question: Can you TLDR how the static analysis stuff works? Does it help psalm somehow?
There was a problem hiding this comment.
It doesn't help psalm, it just checks that our type assertions are correct.
Here, we check eg. if after Option#isSome() returns true, psalm understands that we have a Some instance by passing it to a function that expects a Some instance as its first arg. Since psalm doesn't generate an error for this, it means our type assertions are valid.
No, because composer won't even pickup the new version since it won't satisfy the defined php constraints. |
Same as prewk/option#17 * dropping support for `PHP < 8.1.0` * simplify phpdoc types, making `Result` the only source of truth for most methods * Turning `Ok<T, E>` into `Ok<T>` and `Err<T,E>` into `Err<E>` * added some static analysis assert for `Result#isOk()` and `Result#isErr()` * added a bunch of missing methods on `Result` from their Rust counterparts * migrating from phpspec to PHPUnit * added a Makefile to help during development * added a coding standard via `easy-coding-standard` * added mutation testing coverage via `infection` * added some static analysis tests * modernize github actions setup * added dependabot config * upgrading all dependencies
Same as prewk/option#17 * dropping support for `PHP < 8.1.0` * simplify phpdoc types, making `Result` the only source of truth for most methods * Turning `Ok<T, E>` into `Ok<T>` and `Err<T,E>` into `Err<E>` * added some static analysis assert for `Result#isOk()` and `Result#isErr()` * added a bunch of missing methods on `Result` from their Rust counterparts * migrating from phpspec to PHPUnit * added a Makefile to help during development * added a coding standard via `easy-coding-standard` * added mutation testing coverage via `infection` * added some static analysis tests * modernize github actions setup * added dependabot config * upgrading all dependencies
Same as prewk/option#17 * dropping support for `PHP < 8.1.0` * simplify phpdoc types, making `Result` the only source of truth for most methods * Turning `Ok<T, E>` into `Ok<T>` and `Err<T,E>` into `Err<E>` * added some static analysis assert for `Result#isOk()` and `Result#isErr()` * added a bunch of missing methods on `Result` from their Rust counterparts * migrating from phpspec to PHPUnit * added a Makefile to help during development * added a coding standard via `easy-coding-standard` * added mutation testing coverage via `infection` * added some static analysis tests * modernize github actions setup * added dependabot config * upgrading all dependencies
PHP < 8.1.0Optionthe only source of truth for most methodsOption#isSome()andOption#isNone()(closes Php storm flags unwrap method as unreachble statement #16)Option#inspect()andOption#filter()from their Rust counterpartseasy-coding-standardinfection