iTop
iTop copied to clipboard
N°8796 - Add pre-commit hooks for auto PHP code style fixing
Base information
| Question | Answer |
|---|---|
| Related to a SourceForge thead / Another PR / Combodo ticket? | N°8796 |
| Type of change? | Code quality |
Objective (enhancement)
In a previous PR, we introduced the PHP code style tool (PHP CS Fixer), a re-formatting of the code base and a comformity check in the CI.
This PR aims at easing developer's experience by automatically running the PHP code style fixer on files about to be committed. Avoiding to manually launch the tool or to make a "Fix CI" commit after pushing non-comform code.
Proposed solution
What has been done:
- Add a Git hooks manager running with PHP (CaptainHook)
- Run PHP-CS-Fixer on
pre-commithook to fix code style of staged PHP files- Why use a custom PHP script instead of the "native" PHP-CS-Fixer command in the
<ITOP>/captainhook.json?- When running PHP-CS-Fixer native command on the
pre-commithook it would either:- In case of
check, block the commit, which was good but forced the developer to manually run PHP-CS-Fixer, then re-commit => Not OK - In case of
fix, commit files anyway, then fix them for a second commit => Not OK
- In case of
- Using a custom PHP script, we can run the
fixcommand on files, then re-stage them for the current commit, which makes it a transparent operation for the developer
- When running PHP-CS-Fixer native command on the
- Why use a custom PHP script instead of the "native" PHP-CS-Fixer command in the
How to use it:
- Install the PHP code style tool (PHP-CS-Fixer), must be done only for the 1st run
- Go to
<ITOP>/tests/php-code-stylefolder - Install the dependencies
composer install
- Go to
- Install the Git hooks manager (CaptainHook), must be done only for the 1st run
- Go to
<ITOP>folder - Duplicate
<ITOP>/captainhook.config.json.distinto<ITOP>/captainhook.config.json - Edit it to keep only 1
php-pathline and adjust it to your environement (It's important to change this BEFORE running the next command, otherwise the Git hook will be installed with an incorrect path to PHP) - Install the dependencies AND the git hooks on your environment
composer install
- Go to
- Make some changes on PHP files
- Commit these PHP files
- In the Git history, you should see that these files were modified to match PHP code style if they didn't already
Checklist before requesting a review
- [X] I have performed a self-review of my code
- [X] I have tested all changes I made on an iTop instance
- [X] I have added a unit test, otherwise I have explained why I couldn't => No test coverage on this as it is not included in the package tested by the CI.
- [X] Is the PR clear and detailed enough so anyone can understand digging in the code?
Things to keep in mind
- [ ] The Git hook manager has been added to Composer through
require-devjust like for some other dependencies (e.g. "web profiler"), it should be up to the Factory to clean these "dev dependencies" when building the final package. - [ ] Maybe we should update the README.md or add a developers section to explain how to set-up a development environment.
Works on Linux with a PHP not located at default location