Add recommendations & warnings#282
Conversation
…recommendation checks
theofidry
force-pushed
the
feature/warnings
branch
from
e5f56a2 to
154380e
Compare
| prompted for the passphrase. | ||
| prompted for the passphrase unless you are not in an interactive environment. | ||
|
|
||
| This setting will be ignored if no [key][key] has been provided. |
There was a problem hiding this comment.
[key][key] ?
what this MD syntax stands for ?
There was a problem hiding this comment.
Instead of doing:
[link text](link)You can do:
[link text][anchor]
[anchor]: linkWhich allows you to group all the links in one place (here at the bot of the file) and re-use a given link in several places.
It looks funny here because coincidentally the anchor name is the same as the linked text
There was a problem hiding this comment.
awesome ;) TIL ;) thanks !
| // TODO: no longer accept strings & document BC break | ||
| if (false === isset($raw->algorithm)) { | ||
| return Phar::SHA1; | ||
| return self::DEFAULT_SIGNING_ALGORITHM; |
There was a problem hiding this comment.
what if this algorithms is not one available in get_phar_signing_algorithms ?
There was a problem hiding this comment.
get_phar_signing_algorithms() would need to be updated - it takes all the algorithms allowed by Phar right now, (which is unlikely to happen often so maintaining that should be ok)
| Assertion::true( | ||
| defined('Phar::'.$algorithm), | ||
| sprintf( | ||
| 'The signing algorithm "%s" is not supported by your current PHAR version.', |
There was a problem hiding this comment.
wow, to be honest, I would not expect that, after we checked our algorithm is in get_phar_signing_algorithms :|
There was a problem hiding this comment.
Me neither, but it would be possible for example that a new PHP version introduce a new signing algorithm, in which case get_phar_signing_algorithms () would have it, but the PHP version you are using does not allow it.
Just to be clear: there is no change of behaviour here in the sense it would just fail as before, the difference is the failure would happen with a more friendly error message
| 'finder', | ||
| ]; | ||
| private const PHP_SCOPER_CONFIG = 'scoper.inc.php'; | ||
| private const DEFAULT_SIGNING_ALGORITHM = Phar::SHA1; |
There was a problem hiding this comment.
are we sure SHA1 is always available? asking, as for other algorithms, we check if it's available for our installation of Phar ext...
There was a problem hiding this comment.
Good question, worth checking even more so since I'd like to check if it's worth pushing for a better signing algorithm
| * | ||
| * @return <string, int> | ||
| */ | ||
| function get_phar_signing_algorithms(): array |
There was a problem hiding this comment.
wow, nothing like that as native PHP function ?
There was a problem hiding this comment.
nevermind got it: no I wish that was par of the PHP phar extension as well as one or two of the functions above... but it's probably not gonna happen any time soon and didn't bother to try to push for it yet either
2 participants
Related to #123.
The current implementation only adds the API at the
Configurationlevel. Some TODOs have been changed as well in favour of that new system but I am certain there is more "recommendations" that could be added.To completely close #123 more work needs to be done on the UI for the user to be able to read them.
Other notable changes:
isPrivateKeyPrompt()topromptPrivateKey(). The old method is kept for BC but is marked as deprecated.Pharclass)algorithm,keyandkey-passcombinationsdatetime_formatwhich is deprecated todatetime-format