-
Notifications
You must be signed in to change notification settings - Fork 1
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
Additional auto fixables #5
base: main
Are you sure you want to change the base?
Conversation
Fix #6 now too (except for lists? since we kind of expect lists to have the same ordering, that's the whole reason why list is used, isn't it?) |
@orklah if you have a few mins I'd really appreciate some feedback on this |
Yeah sorry, it's on my list, I'll get to it soon |
Oh. I just realized you didn't see my first round of comments because I didn't publish them :( sorry. I thought you responded to it earlier but it was just a response to my new issue. You'll see duplicate comments or outdated comments, you can resolve them directly |
0163e37
to
7554339
Compare
7554339
to
1830a62
Compare
1830a62
to
dc80684
Compare
Changed various things to handle more common cases and excluded things that won't work. Overall works quite well, feedback appreciated so this can get merged eventually |
It's really became a complex piece of code. I'd be way more at ease if we could make some kind of automated check that everything is correct. Not necessarily a whole test suite, but some script we can run to check whether the plugin is doing its job |
I can add that eventually and merge it then, for now could you please merge #7 and release so the existing plugin can be used as is with psalm v5 at least? |
Done! Make sure to report any error while running it, the lack of tests on those plugins makes this kind of change random :p |
<pluginClass class="Orklah\StrictEquality\Plugin"> <strictEqualityFromDocblock value="true" /> </pluginClass>
e330527
to
0036f0a
Compare
Fix: #3