Add recommendations#285
Conversation
…re accessed using the key constants
theofidry
force-pushed
the
feature/add-recommendations
branch
from
a103df1 to
06740e4
Compare
|
/cc @keradus |
| ### Force auto-discovery (`auto-discovery`) | ||
| ### Force auto-discovery (`force-autodiscovery`) | ||
|
|
||
| The `auto-discovery` (`bool` default `false`) setting forces Box to attempt to find which files to include even though |
There was a problem hiding this comment.
shall be renamed as well?
change of input name (CLI param, param in config file) is a BC breaker
There was a problem hiding this comment.
Indeed should be renamed.
It is indeed a BC break but this is fine since it was on master but not released yet
| private const PHP_SCOPER_CONFIG = 'scoper.inc.php'; | ||
| private const DEFAULT_SIGNING_ALGORITHM = Phar::SHA1; | ||
|
|
||
| private const ALGORITHM_KEY = 'algorithm'; |
There was a problem hiding this comment.
I would suggest to put them into separated, non-instanceable class, or use prefix (instead of suffix) for easier distinguishability
There was a problem hiding this comment.
I still have mixed feelings about it as well to be honest, but for now I think I prefer to keep it as it is
| ? Removing the Composer dump artefacts | ||
| ? No compression | ||
| ? Setting file permissions to 0755 | ||
| ? Setting file permissions to 0754 |
There was a problem hiding this comment.
out of curiosity,
why the change ?
There was a problem hiding this comment.
Because I wanted to configure the chmod still but not raising a recommendation (it was the case here since the value set was the same as the default one)
|
Thanks for the review @keradus :) |
2 participants
Closes #123
auto-discoverytoforce-autodiscoverystdClassinstance properties of the raw configuration via constants for the keys instead of arbitrary stringsnullor the default value. The recommendation being that the setting can be completely ignorednew InvalidArgumentException()statements with their respective TODOs in favour of relying onAssertionfrombaberlei/assert