Skip to content
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

Apply suggestion by @mdeweerd to ensure pre-commit will pass. #32651

Merged

Conversation

atm-florianm
Copy link
Contributor

FIX pre-commit failing on a debug bar class

As discussed in #32621, there are some code standards issues with the DebugBar in v17, which cause the pre-commit hook to fail even though these files are entirely unaffected by the PR.

I am opening this PR instead to separate this issue from the original one.

@atm-florianm
Copy link
Contributor Author

atm-florianm commented Jan 14, 2025

PHP Fatal error: $GLOBALS can only be modified using the $GLOBALS[$name] = $value syntax in htdocs/includes/symfony/var-dumper/Tests/CliDumperTest.php on line 512

@eldy Should I attempt to fix that as well, or is it better to leave it alone for the time being?

@mdeweerd
Copy link
Contributor

PHP Fatal error:  Array and string offset access syntax with curly braces is no longer supported in htdocs/includes/tecnickcom/tcpdf/include/barcodes/pdf417.php on line 881
Errors parsing htdocs/includes/tecnickcom/tcpdf/include/barcodes/pdf417.php

We either ignore this like so:

- id: php-lint
exclude:
(?x)^(htdocs/includes/symfony/var-dumper/Tests/.*)$

Or we ensure that an older php version is installed in the pre-commit setup (corresponding to the maximum version supported by the corresponding dollibarr version).
Get the version by getting it from htdocs/install/check.php - currently in develop: "$arrayphpmaxversionwarning = array(8, 3, 0);"

@mdeweerd
Copy link
Contributor

mdeweerd commented Jan 14, 2025

@atm-florianm I made a PR to your branch which installs the maximum supported PHP version before running pre-commit.

This way ci work and will only show issues in the appropriate dolibarr branches where external libraries may already have been updated.

EDIT:
Or, I can make the PR directly to dolibarr as my change fixes the issue by itself in 17.0.
EDIT2:
I went ahead and created #32653 .

@eldy eldy added the PR to fix - Conflict or CI error to solve The PHP unit tests return something wrong. Check details to know what to fix or solve the conflicts. label Jan 14, 2025
@eldy eldy merged commit fe6d59c into Dolibarr:17.0 Jan 15, 2025
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR to fix - Conflict or CI error to solve The PHP unit tests return something wrong. Check details to know what to fix or solve the conflicts.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants