Restore php 5.3 support #51

glensc · 2021-03-13T16:02:30Z

Originally, the PHP 5.3 was removed, because Travis misconfiguration (32855d5). However, seems most PHP 5.3 code is still there. This will attempt to restore such compatibility.

The --json support is not added, see notes below.

This is needed for projects that support old versions, example:

https://github.com/zf1s/zf1

As the version still won't be tested in CI, such use will be at their own risk, as this project doesn't CI test those versions.

PS: once this project CI is migrated to GitHub actions and setup-php, then PHP 5.3 support can be enabled in Ci. Also, php 5.3 can be enabled in Travis, likely the original commit author did not know-how.

PS2: This can be seen in action here: zf1s/zf1#66

glensc · 2021-03-14T08:29:00Z

Side note, the PHP version check never worked, as it was not expected that the file must conform to the syntax for the PHP version. In other words, if you want the script to print error message under (for example) php 4.3.0, your entry point PHP script must be a valid syntax for PHP 4.3.0:

> parallel-lint --exclude vendor --exclude tests/Zend/Loader/_files/ParseError.php . '--checkstyle'
PHP Parse error:  syntax error, unexpected '[' in /home/runner/work/php-zf1s/php-zf1s/vendor/php-parallel-lint/php-parallel-lint/parallel-lint on line 38

https://github.com/glensc/php-zf1s/runs/2105646130?check_suite_focus=true

glensc · 2021-03-14T08:40:45Z

Funnily, the most breaking change was made in the entry point of this application, which should have been kept compatible with any PHP version to say out a nice error message:

Restore php 5.3 support #51 (comment)

glensc · 2021-03-14T10:39:25Z

Note the --json doesn't work, silently does nothing, but it's okay as it's documented in the readme that requires php 5.4:

$ php53 ./parallel-lint  --json parallel-lint
{"phpVersion":80001,"hhvmVersion":"","parallelJobs":10,"results":{}}

$ php53 ./parallel-lint -p php53  --json parallel-lint
{"phpVersion":50329,"hhvmVersion":"","parallelJobs":10,"results":{}}

glensc · 2021-03-14T10:43:05Z

This can be seen in action here:

https://github.com/zf1s/zf1/pull/66/files#diff-2a4f8e29f68dc079964997915a341ad6d80d34989aadac4db9a7e26772b6cb28R185

I've included screenshot there as well, because before merge the syntax error is intended to be removed.

composer.json

glensc · 2021-03-20T17:47:20Z

Any chance for merge and release, before some big and breaking changes come in?

This way would have at least one release that can be used in php 5.3 projects and I can finish up it in my project:

Use staabm/annotate-pull-request-from-checkstyle to report violatons in GitHub zf1s/zf1#66

roelofr · 2021-03-22T19:43:34Z

This might be a bit blunt, but is this a good idea?

PHP 5.3 has been EOL since August 14th, 2014. Maybe we shouldn't re-open and re-support dead horses?

glensc · 2021-03-22T20:34:49Z

@roelofr yes, this is blunt, as outlined elsewhere in this project's comments, this project is useful for other projects, whose version support is their own consideration.

The changes here are very minimal, in fact, the short array syntax should be a separate PR, but I expected this PR to go smooth.

you support 5.4, which equally also very dead in such a sense.

and also, the original commit that dropped 5.3, was due to wrong reasons, also outlined in PR body.

roelofr · 2021-03-22T21:21:40Z

Okay, sorry, no offence intended.

Should this be part of #46? Add tests to ensure php 5.3 still works?

glensc · 2021-03-22T22:56:52Z

@roelofr I don't expect any actual support, for me it's fine with just these changes released and no other guarantees. For example, the json output part is not working, but it's probably possible to make it work.

From the project manager's point of view, it's probably best to add CI integration too. But then need to update the readme as well that 5.3 is supported.

It's a kind of a chicken-egg problem right now with the CI, because without CI I can't test and without php 5.3 support one can't develop CI. so, perhaps deal with that once this and CI tasks are merged?

glensc · 2021-03-22T22:59:29Z

I looked at #46, and it describes that it depends on code-restructuring task... in my opinion, it's way too big to chew, as it will take ages to get there...

grogy · 2021-03-27T15:12:31Z

PHP 5.3 is old, really too much old. On the other hand, I know more live projects with PHP < 5.4 with sometimes changes.

Codebase does not use new features. Ok - it makes sense to go back to support.

Please, can you:

add support to Github Actions?
change readme The application is officially supported for use with PHP 5.4 to 8.0.

glensc · 2021-03-27T19:10:04Z

@grogy added CI and updated readme. And it passes, wohoo!

and so is 5.4 old, the only notable difference with 5.4 is the short array and some newer interfaces. But 5.3 was some groundwork from what things evolved further. I guess it stayed because it was the first version with namespaces, and php without namespaces support today just not possible in the composer ecosystem.

thanks for accepting this PR.

jrfnl · 2021-03-28T02:59:11Z

Would be good to see this actually tested in combination with #55 which re-enables the running of the unit and integration tests.

This way the polyfill won't be executed when php-parallel-lint is loaded as composer dependency but in application only.

jrfnl

LGTM.

Only thing I'm wondering about is whether the generation of the phar in the GH Actions workflow should now also be done on PHP 5.3, though between PHP 5.3 and 5.4, I don't suppose there would be that much of a difference (if any).

jrfnl · 2021-03-28T12:20:18Z

FYI: I've just done a test run with this PR and PR #55 combined and depending on whichever PR gets merged first, the other PR will need to be adjusted to add the following step to the test job, between the setup-php and composer-install steps in the Test workflow to allow for that workflow to run properly:

      # Remove PHPCS as it has a minimum PHP requirements of PHP 5.4 and would block install on PHP 5.3.
      - name: 'Composer: remove PHPCS'
        if: ${{ matrix.php < 5.4 }}
        run: composer remove --dev squizlabs/php_codesniffer --no-update

@glensc Shall I just add that change to PR #55 in anticipation of both PRs being merged ?

glensc · 2021-03-28T20:55:42Z

@jrfnl I can rebase once the other dependency is merged. altho I don't see why not do the other way around. The CI knowledge is bigger in the other PR.

jrfnl · 2021-03-28T21:08:36Z

@glensc I've adjusted the other PR in a way that a build against PHP 5.3 will pass and confirmed this with a test build, so either one of these PRs getting merged first will be fine now.
There might be an imaginary merge conflict after one of the merges, but there should be no real conflict between the two PRs and the order in which they get merged is not really relevant (as a full CI test has been done and passes).

glensc · 2021-03-30T21:01:51Z

@grogy merge & release? or what is this waiting for?

grogy · 2021-03-30T21:10:52Z

Thanks for the changes. I wanted first to check and merge #55 with unit testing all PHP versions. It is merged now. Can you please rebase the current master? When the CI success then we can immediately merge it.

jrfnl · 2021-03-30T21:18:19Z

@grogy FYI: as I said above - I have already done a test run with #55 and this PR combined, so it should be fine.

glensc · 2021-03-30T22:39:29Z

Super! Can this be released too before other big PR's?

grogy · 2021-03-31T03:51:59Z

Yes, I mean it is a good point. I will test today / tomorrow ;)

As Parallel Lint supports PHP 5.3 again since [PR 51](php-parallel-lint/PHP-Parallel-Lint#51), it would be helpful for the Console Color repo to also support PHP 5.3.

As Parallel Lint supports PHP 5.3 again since [PR 51](php-parallel-lint/PHP-Parallel-Lint#51), it would be helpful for the Console Color repo to also support PHP 5.3. This restores the PHP 5.3 minimum version to the same as before the version drop in de0bf63

As Parallel Lint supports PHP 5.3 again since [PR 51](php-parallel-lint/PHP-Parallel-Lint#51), it would be helpful for the Highlighter repo to also support PHP 5.3. This restores the PHP 5.3 minimum version to the same as before the version drop in 02b6aa6

As Parallel Lint supports PHP 5.3 again since [PR 51](php-parallel-lint/PHP-Parallel-Lint#51), it would be helpful for the Console Color repo to also support PHP 5.3. This restores the PHP 5.3 minimum version to the same as before the version drop in de0bf63

As Parallel Lint supports PHP 5.3 again since [PR 51](php-parallel-lint/PHP-Parallel-Lint#51), it would be helpful for the Highlighter repo to also support PHP 5.3. This restores the PHP 5.3 minimum version along the same lines as before the version drop in 02b6aa6 The PHP 5.3.2 minimum is in-line with the PHP Console Color minimum version for PHP 5.3. Includes: * Enabling a build against PHP 5.3 in the GH Actions workflow. * Adjusting the `testVersion` for the PHPCompatibility checks to `5.3-` (5.3 and higher).

This was referenced Mar 13, 2021

Restore php 5.3 JakubOnderka/PHP-Parallel-Lint#159

Closed

Use staabm/annotate-pull-request-from-checkstyle to report violatons in GitHub zf1s/zf1#66

Merged

glensc force-pushed the php53-v2 branch 2 times, most recently from 434e837 to 665b384 Compare March 14, 2021 08:19

glensc changed the title ~~Restore php 5.3~~ Restore php 5.3 support Mar 14, 2021

glensc force-pushed the php53-v2 branch from 5ca61b2 to 0d1f9fc Compare March 14, 2021 08:40

glensc marked this pull request as ready for review March 14, 2021 10:43

jrfnl reviewed Mar 14, 2021

View reviewed changes

composer.json Outdated Show resolved Hide resolved

glensc force-pushed the php53-v2 branch from 0d1f9fc to c003785 Compare March 20, 2021 17:46

glensc added 4 commits March 27, 2021 21:04

Add polyfill for \JsonSerializable

d7b8921

Use traditional array syntax

4dc2e43

Allow loading script with php 5.3

13d8caf

Allow php 5.3 in composer

c699e2d

glensc force-pushed the php53-v2 branch from c003785 to c699e2d Compare March 27, 2021 19:04

Add php 5.3 to CI and listed as supported

1acda5a

Load polyfill from application, not from composer'

3f2cc26

This way the polyfill won't be executed when php-parallel-lint is loaded as composer dependency but in application only.

jrfnl approved these changes Mar 28, 2021

View reviewed changes

jrfnl mentioned this pull request Mar 28, 2021

GH Actions: improve workflows #55

Merged

grogy merged commit b2643dc into php-parallel-lint:master Mar 30, 2021

grogy mentioned this pull request Mar 31, 2021

New version - 1.3 #56

Closed

jrfnl mentioned this pull request May 8, 2021

Split files for PSR-4 compliance #45

Merged

grogy mentioned this pull request Dec 1, 2021

More maintainers? #32

Closed

jrfnl added a commit to jrfnl/PHP-Console-Color that referenced this pull request Dec 5, 2021

Support PHP 5.3

84b4915

As Parallel Lint supports PHP 5.3 again since [PR 51](php-parallel-lint/PHP-Parallel-Lint#51), it would be helpful for the Console Color repo to also support PHP 5.3.

jrfnl mentioned this pull request Dec 7, 2021

Bring back support for PHP 5.3 php-parallel-lint/PHP-Console-Color#22

Merged

jrfnl mentioned this pull request Dec 12, 2021

GH Actions: PHP 5.3 was missing from release workflow #79

Merged

jrfnl mentioned this pull request Dec 25, 2021

Bring back support for PHP 5.3 php-parallel-lint/PHP-Console-Highlighter#33

Merged

jrfnl added the Type: enhancement label Mar 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restore php 5.3 support #51

Restore php 5.3 support #51

glensc commented Mar 13, 2021 •

edited

Loading

glensc commented Mar 14, 2021

glensc commented Mar 14, 2021

glensc commented Mar 14, 2021

glensc commented Mar 14, 2021

glensc commented Mar 20, 2021

roelofr commented Mar 22, 2021

glensc commented Mar 22, 2021

roelofr commented Mar 22, 2021

glensc commented Mar 22, 2021

glensc commented Mar 22, 2021

grogy commented Mar 27, 2021

glensc commented Mar 27, 2021

jrfnl commented Mar 28, 2021

jrfnl left a comment

jrfnl commented Mar 28, 2021 •

edited

Loading

glensc commented Mar 28, 2021

jrfnl commented Mar 28, 2021

glensc commented Mar 30, 2021

grogy commented Mar 30, 2021

jrfnl commented Mar 30, 2021

glensc commented Mar 30, 2021

grogy commented Mar 31, 2021

Restore php 5.3 support #51

Restore php 5.3 support #51

Conversation

glensc commented Mar 13, 2021 • edited Loading

glensc commented Mar 14, 2021

glensc commented Mar 14, 2021

glensc commented Mar 14, 2021

glensc commented Mar 14, 2021

glensc commented Mar 20, 2021

roelofr commented Mar 22, 2021

glensc commented Mar 22, 2021

roelofr commented Mar 22, 2021

glensc commented Mar 22, 2021

glensc commented Mar 22, 2021

grogy commented Mar 27, 2021

glensc commented Mar 27, 2021

jrfnl commented Mar 28, 2021

jrfnl left a comment

Choose a reason for hiding this comment

jrfnl commented Mar 28, 2021 • edited Loading

glensc commented Mar 28, 2021

jrfnl commented Mar 28, 2021

glensc commented Mar 30, 2021

grogy commented Mar 30, 2021

jrfnl commented Mar 30, 2021

glensc commented Mar 30, 2021

grogy commented Mar 31, 2021

glensc commented Mar 13, 2021 •

edited

Loading

jrfnl commented Mar 28, 2021 •

edited

Loading