search

DataTables / Editor-PHP

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

Refactor query variable names #35

Closed mvorisek closed 2 months ago

mvorisek commented 3 months ago

non-functional improvement, do not assign scalar count value to the same variable which hold the query object

mvorisek commented 3 months ago

the PPH 5.4 CI failure is unrelated - https://github.com/docker-library/php/issues/1522

mvorisek commented 2 months ago

@AllanJard can this refactoring PR be merged?

Also one thing related to the https://editor.datatables.net/download/download?type=php release process - .gitignore and .php-cs-fixer.dist.php files should be present (files with dot in general) as user might want to run composer install and PHP CS Fixer on the release locally after some local changes.

AllanJard commented 2 months ago

.gitignore and .php-cs-fixer.dist.php files should be present

Actually, I've had negative feedback including the dot files before. I used to do that, but removed then when I got some "robust" feedback that they shouldn't be included.

I'm okay with them not being included at the moment - the download packages are not meant to be a development environment, the git repo exists for that. I'll keep an eye out for more feedback on this though - thank you.

mvorisek commented 2 months ago

Do you mean .gitignore specifically? Maybe there were some other files, but the .gitignore should be present as when the release is put into VCS and composer update is run, then composer files are not ignored.

AllanJard commented 2 months ago

Yeah - it was a while back, but as I recall the issue reported be a few people was that they were upset that my .gitignore was being used and thus caused some files the had expected to be committed not to be. Or something like that. I think that the downloaded PHP Editor demo package would normally be merged into the developers own project, so I'm not sure .gitignore would be relevant in such a case.

I'll add one vote to the pile for including .gitignore :)