Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

PHP CS Fixer #11

Merged
merged 4 commits into from
Jan 10, 2023
Merged

PHP CS Fixer #11

merged 4 commits into from
Jan 10, 2023

Conversation

phpfui
Copy link
Contributor

@phpfui phpfui commented Dec 30, 2022

Added files to run PHPCSFixer and run it.

Due to very old versions of PHP being used in this project, the modern version of PHP CS Fixer can not be included in composer.json. But since this is just a dev tool, it does not matter, as it is just run on a local machine and changes are checked in.

To run, you must install PHPCSFixer globally (or in another project, or temporarily) then run phpcsfixer fix from the project root directory. Check in any changes.

To change the rules, edit the file PhpCsFixer.php in the project root.

Get this merged and I can add PHPStan next.

@zbateson
Copy link
Owner

zbateson commented Jan 5, 2023

Hi @phpfui --

This is great, thanks again! Some of the rules I'd rather keep off -- if I change them tho I assume my code won't be reverted back tho. Could we instead have a pull request just configuring PHPCSFixer? As you have it is fine, and I'll play with it and adjust the rules I don't want, run it and commit it fixed?

@phpfui
Copy link
Contributor Author

phpfui commented Jan 6, 2023

So basically here is how PHP-CD-Fixer works: You specify rules, then run the fixer. It modifies the code to the rules you specify. Then you check in the modified code. This is what I did. I did not do any hand modification. I have also set up git hooks to do this automatically and update the current commit when checking in code. Works well. I can add that as well if you would like, but people would have to configure git locally to do that. There may be a package that might automate that, but have not investigated. Also Linux vs Windows dev envs can get a bit tricky.

What you want to do is modify the rules, the rerun the fixer, then check in the new code. You may want to revert the run I did, just to be 100% sure nothing was lost, but I don't remember doing any hand mods.

There is also a great tool here You should be able to import and export the PhpCsFixer.php file in the project root.

So merge this, change the rules to your specifications, run the fixer, check in the code.

The only other thing we should probably do is to just use one master copy of PhpCcFixer.php in the mb-wrapper package. Since this package is required by the other two, it will always be present and in a known location relative to the current project (vendor/zbateson/mb-wrapper/PhpCsFixer.php). Then we don't have to maintain a version in each package.

Hope this helps clear things up.

@phpfui
Copy link
Contributor Author

phpfui commented Jan 6, 2023

Easy to revert the last commit. So merge and then fix up PhpCsFixer.php to your liking. Easy to revert things, change the config and redo. Once you settle on the right format, I can then put in a PR to use the mb-stream version, then we don't have to maintain separate files. Use this for all my projects and it works great.

@zbateson
Copy link
Owner

zbateson commented Jan 6, 2023

Aah, right -- revert makes sense. Keeping it in mb-wrapper also sounds good, I'll look into that.

For running it, I might configure it to run with 'phing' on mail-mime-parser (I use it to release docs for new versions) and keep it manual on here and mb-wrapper. They see much fewer releases/commits, so can worry about it if that changes. I'll give it some thought.

Thanks again

@zbateson zbateson merged commit 0687897 into zbateson:master Jan 10, 2023
@phpfui phpfui deleted the PHPCSFixer branch January 10, 2023 20:33
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants