Support html+php #22
Support html+php #22
Conversation
| @@ -7,6 +7,7 @@ jobs: | |||
| phpstan: | |||
| name: PHPStan | |||
| runs-on: Ubuntu-20.04 | |||
| continue-on-error: true | |||
There was a problem hiding this comment.
why continuing on error ? If some setup step fails, further steps will also fail in some non-understandable way, as they assume that the setup is done.
There was a problem hiding this comment.
Are you sure?
I though this meant: "If one job fails, please continue with the others jobs anyways"
Ie, it will always run phpstan even if php-cs-fixer fails.
There was a problem hiding this comment.
this is a setting of the job itself. It is about failed steps, not about failed jobs. One job does not impact other jobs, except for 2 reasons:
- when using
needsto declare a dependency between jobs (a job is skipped if its dependencies failed) - when using a matrix without
fail-fast: false, a failed job in the matrix cancels other jobs of the matrix (thatfail-fast: truedefault in github actions is quite annoying IMO. I'm always changing it, as I find value in seeing whether tests on a PHP version are working even if another PHP version broke)
| $contents = 'class Foobar {'.$contents.'}'; | ||
| } | ||
|
|
||
| // Allow us to use "..." as a placeholder |
There was a problem hiding this comment.
what if we have some places using an actual spread operator ?
There was a problem hiding this comment.
We do that once in the symfony docs. So I saw this benefit is greater than the downside. But your comment is valid and it is not the best solution.
There was a problem hiding this comment.
maybe a better heuristic would be to replace only ...; (which cannot be a valid usage of either a variadic parameter definition or a spread operator). I don't know whether we have other kind of usages of the placeholder in the doc.
There was a problem hiding this comment.
Yeah, sure. ...;, ...) and ...,. That would cover most I think.
| // Allow us to use "..." as a placeholder | ||
| $contents = str_replace('...', 'null', $contents); | ||
|
|
||
| $lines = explode(PHP_EOL, $contents); |
There was a problem hiding this comment.
Could you help me understand why?
There was a problem hiding this comment.
because the doc source uses LF line endings, so this code would fail if a contributor tries running it on Windows as it would not explode lines properly
Thiis will fix #19