search

DataTables / Editor-PHP

PHP server-side libraries for Editor
Other
35 stars 22 forks source link

Fix invalid phpdoc #17

Closed mvorisek closed 1 year ago

mvorisek commented 2 years ago

fixes most phpstan level 2 issues

and upgrade phpstan to level 6 to prevent code degradation from new contributions

no API change

mvorisek commented 2 years ago

@AllanJard can these fixes be merged?

there should be no functional change except fixed imports in several catch clauses like in https://github.com/DataTables/Editor-PHP/pull/17/files#diff-01b189458d10702351d63e3a6e1e0d5e9344feae55742cceab46b48cc5ddba6fR102

AllanJard commented 2 years ago

Sorry - I haven't had a chance to review the yet I'm afraid. We seem to be having a huge influx of support requests for some reason at the moment. I'll review as soon as possible :-)

mvorisek commented 2 years ago

@AllanJard is now a better time? We get a lot of phpstan issues because of phpdoc issues fixed with this PR. Thanks in advance.

AllanJard commented 2 years ago

Apologies - I've got a massive backlog of stuff at the moment and haven't been able to prioritise this like I want to. It is very much on my radar though and everything looks fine on a quick scan.

AllanJard commented 2 years ago

This is in part a note to myself for when I get to looking at this - this thread in the forum notes an issue with the abstract clarification from 6ce4b0a9fe8699ec178e89a17fbf7442a523eb84. It is the right thing to do, but it introduced an error with 5.6.0.

Although 5.6 is EOL and hasn't been supported by PHP since 2018, many are stuck with it. I'm concerned about this, so as a note to me - check changes with back to 5.4, which is what we say we currently support.

mvorisek commented 2 years ago

@AllanJard is it now a good time to fix the phpdoc documentation issues?

The correct phpdocs are important for static analysis for dependent projects.

They are several conflicts since the last push, please let me know if we can coordinate the merge soon, and I will fix them. Thanks.

AllanJard commented 2 years ago

Apologies - I'm just a little bit swamped! I need to go over them all with a linter when I get a chance. Not yet sure when that will be.

mvorisek commented 1 year ago

@AllanJard What to do with this PR? When I will fix the conflicts (rebase on the latest master), can I expect it to be merged then?

AllanJard commented 1 year ago

I'm really sorry this has slipped by me. There is just so much going on. I'm working on the 2.2 cycle at the moment, so if you are willing to rebase now, that would be great.

mvorisek commented 1 year ago

@AllanJard once done, may I ask for one DT Editor license for free in exchange for this work?

AllanJard commented 1 year ago

Yes, that seems like a fair trade :-)

mvorisek commented 1 year ago

@AllanJard PR is done

AllanJard commented 1 year ago

Amazing - thank you.