ramsey / conventional-commits

:yo_yo: A PHP library for creating and validating commit messages according to the Conventional Commits specification. Includes a CaptainHook plugin!
MIT License
178 stars 19 forks source link

fix: phpstan error when memory_limit is 128M #65

Closed hussainweb closed 1 year ago

hussainweb commented 1 year ago

Description

PHPStan results in a memory limit error when I run on a machine with default the default memory_limit of 128M (or it could be my OS). This small change ensures that PHPStan works fine on machines with this limit.

Motivation and context

My CLI runs on the default memory_limit of 128M and I noticed these errors in development. I could set the memory_limit in my ini file, of course, but this is inconvenient. Besides, I believe the change here is simple and should not affect anything else.

How has this been tested?

By manually running composer dev:analyze:phpstan after clearing the result cache (vendor/bin/phpstan clear-result-cache)

Types of changes

PR checklist

codecov[bot] commented 1 year ago

Codecov Report

Merging #65 (b7292b1) into main (75d47b3) will not change coverage. The diff coverage is n/a.

Additional details and impacted files [![Impacted file tree graph](https://codecov.io/gh/ramsey/conventional-commits/pull/65/graphs/tree.svg?width=650&height=150&src=pr&token=GRB47TNJB9&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ben+Ramsey)](https://codecov.io/gh/ramsey/conventional-commits/pull/65?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ben+Ramsey) ```diff @@ Coverage Diff @@ ## main #65 +/- ## ========================================= Coverage 98.71% 98.71% Complexity 286 286 ========================================= Files 37 37 Lines 779 779 ========================================= Hits 769 769 Misses 10 10 ```
hussainweb commented 1 year ago

It seems phpstan doesn't accept -1 as a valid input. I get this error.

✖ composer dev:analyze:phpstan
> phpstan analyse --ansi --memory-limit 0
PHP Warning:  Failed to set memory limit to 0 bytes (Current memory usage is 33554432 bytes) in phar:///Users/hw/work/php/conventional-commits/vendor/phpstan/phpstan/phpstan.phar/src/Command/CommandHelper.php on line 96
Warning: Failed to set memory limit to 0 bytes (Current memory usage is 33554432 bytes) in phar:///Users/hw/work/php/conventional-commits/vendor/phpstan/phpstan/phpstan.phar/src/Command/CommandHelper.php on line 96
Memory limit "0" cannot be set.
Script phpstan analyse --ansi --memory-limit 0 handling the dev:analyze:phpstan event returned with error code 1

If you think 256M is conservative, shall I make it something like 1G or so?

ramsey commented 1 year ago

Interesting. It looks like it's converting it to 0.

256M is fine, though.

hussainweb commented 1 year ago

Ah, sorry, that was the wrong output when I tried 0. But -1 also generates an error. This one:

❯ composer dev:analyze:phpstan
> phpstan analyse --ansi --memory-limit -1

  The "--memory-limit" option requires a value.

analyse [-c|--configuration CONFIGURATION] [-l|--level LEVEL] [--no-progress] [--debug] [-a|--autoload-file AUTOLOAD-FILE] [--error-format ERROR-FORMAT] [-b|--generate-baseline [GENERATE-BASELINE]] [--allow-empty-baseline] [--memory-limit MEMORY-LIMIT] [--xdebug] [--fix] [--watch] [--pro] [--] [<paths>...]

Script phpstan analyse --ansi --memory-limit -1 handling the dev:analyze:phpstan event returned with error code 1

Now I see that it might be interpreting -1 as an option and "-1" might have fixed it. :/ But I think 256M is fine too.

ramsey commented 1 year ago

No worries. If it’s still an issue for anyone else, we can change it then. 🙂