Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow PHP 8 #201

Merged
merged 9 commits into from
Jan 18, 2021
Merged

Allow PHP 8 #201

merged 9 commits into from
Jan 18, 2021

Conversation

MidnightDesign
Copy link
Contributor

No description provided.

composer.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/maglnet/ComposerRequireChecker/pull/201/checks?check_run_id=1252051299#step:7:10

Seems like composer.lock needs an update.

composer update nothing could suffice

@Ocramius Ocramius added dependencies Pull requests that update a dependency file enhancement labels Oct 14, 2020
@Ocramius Ocramius added this to the 2.2.0 milestone Oct 14, 2020
@MidnightDesign
Copy link
Contributor Author

https://github.com/maglnet/ComposerRequireChecker/pull/201/checks?check_run_id=1252051299#step:7:10

Seems like composer.lock needs an update.

composer update nothing could suffice

Ah, of course. I'm not used to having lockfiles in projects I contribute to.

@Ocramius
Copy link
Collaborator

Ha, need to update my own dependencies now :D

@Ocramius
Copy link
Collaborator

I'm not used to having lockfiles in projects I contribute to.

Just to be clear, those are only in place so that tools like phpcs, psalm, phpstan etc do not change randomly and report slightly different results :)

@MidnightDesign
Copy link
Contributor Author

Just to be clear, those are only in place so that tools like phpcs, psalm, phpstan etc do not change randomly and report slightly different results :)

Yeah, obviously. I was thinking about this as a library, when I should have been thinking about it as a project.

@Ocramius Ocramius modified the milestones: 2.2.0, 3.0.0 Nov 8, 2020
@martinssipenko
Copy link
Contributor

Any updates on this? Is there anything I could do to get this done?

@Ocramius
Copy link
Collaborator

I'd say #208 needs to happen first. What we got here is mostly dependency issues, for now.

@martinssipenko
Copy link
Contributor

@Ocramius #208 is merged what am I missing?

@Ocramius
Copy link
Collaborator

@martinssipenko just a rebase needed now :-)

@MidnightDesign
Copy link
Contributor Author

Now we're waiting on webmozarts/glob#21

@Firehed
Copy link
Contributor

Firehed commented Nov 30, 2020

If there's a hold-up there, it seems like there's only one use of that dependency (ComposerRequireChecker\FileLocator\LocateFilesByGlobPattern) which seems like it could probably be factored out. Though I suppose it could have implications on advanced use-cases with custom configs (my gut check says they'd be rare based on the intersection of people that would use this and people with non-autoloaded source code)

@Ocramius
Copy link
Collaborator

If there's a hold-up there

PHP 8 was released last week: I'd call "hold up" when it's February/March :-)

@MidnightDesign
Copy link
Contributor Author

I think @Firehed isn't completely off here. Maybe the dependency could be loosened/removed even if the pull request over there is merged. Fewer dependencies is (almost) always a good thing I think.

@martinssipenko
Copy link
Contributor

webmozarts/glob version 4.2.0 was just erleased with suppport for PHP8. @MidnightDesign can you bump the deps please?

@DanielBadura
Copy link
Contributor

@MidnightDesign could you add PHP8 to the matrix for the phar creation action? Then @OskarStark could test if his issue which he opened the last days is resolved via the phar that gets build in the CI process. I think this would be awesome 🥇

@MidnightDesign
Copy link
Contributor Author

@MidnightDesign could you add PHP8 to the matrix for the phar creation action? Then @OskarStark could test if his issue which he opened the last days is resolved via the phar that gets build in the CI process. I think this would be awesome

You mean in the GitHub Actions config, right? Will do.

Would it be useful to add it to any other workflows? Do Psalm or PHPStan do something differently depending on the version? What about require-checker.yml?

@MidnightDesign MidnightDesign marked this pull request as ready for review January 15, 2021 10:52
@Ocramius
Copy link
Collaborator

require-checker.yml is quite agnostic, but needs to run on 7.x for another while...

composer.json Outdated Show resolved Hide resolved
@OskarStark
Copy link
Contributor

Can you help me out how to test this? Before I just installed the phar via PHIVE 🤔

@Ocramius
Copy link
Collaborator

@OskarStark make a new directory, add a composer.json with a packages section listing @MidnightDesign's branch, add the exact version as dependency and run composer update

@OskarStark
Copy link
Contributor

I used:

{
    "name": "test/foo",
    "type": "project",
    "description": "Test",
    "license": "proprietary",
    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/MidnightDesign/ComposerRequireChecker.git"
        }
    ],
    "require": {
        "maglnet/composer-require-checker": "dev-patch-1"
    }
}

->

Loading composer repositories with package information
Updating dependencies
Lock file operations: 15 installs, 0 updates, 0 removals
  - Locking maglnet/composer-require-checker (dev-patch-1 57ed1dc)
  - Locking nikic/php-parser (v4.10.4)
  - Locking psr/container (1.0.0)
  - Locking symfony/console (v5.2.1)
  - Locking symfony/polyfill-ctype (v1.22.0)
  - Locking symfony/polyfill-intl-grapheme (v1.22.0)
  - Locking symfony/polyfill-intl-normalizer (v1.22.0)
  - Locking symfony/polyfill-mbstring (v1.22.0)
  - Locking symfony/polyfill-php73 (v1.22.0)
  - Locking symfony/polyfill-php80 (v1.22.0)
  - Locking symfony/service-contracts (v2.2.0)
  - Locking symfony/string (v5.2.1)
  - Locking webmozart/assert (1.9.1)
  - Locking webmozart/glob (4.2.0)
  - Locking webmozart/path-util (2.3.0)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 15 installs, 0 updates, 0 removals
  - Downloading webmozart/glob (4.2.0)
  - Syncing maglnet/composer-require-checker (dev-patch-1 57ed1dc) into cache
  - Installing symfony/polyfill-ctype (v1.22.0): Extracting archive
  - Installing webmozart/assert (1.9.1): Extracting archive
  - Installing webmozart/path-util (2.3.0): Extracting archive
  - Installing webmozart/glob (4.2.0): Extracting archive
  - Installing symfony/polyfill-php80 (v1.22.0): Extracting archive
  - Installing symfony/polyfill-mbstring (v1.22.0): Extracting archive
  - Installing symfony/polyfill-intl-normalizer (v1.22.0): Extracting archive
  - Installing symfony/polyfill-intl-grapheme (v1.22.0): Extracting archive
  - Installing symfony/string (v5.2.1): Extracting archive
  - Installing psr/container (1.0.0): Extracting archive
  - Installing symfony/service-contracts (v2.2.0): Extracting archive
  - Installing symfony/polyfill-php73 (v1.22.0): Extracting archive
  - Installing symfony/console (v5.2.1): Extracting archive
  - Installing nikic/php-parser (v4.10.4): Extracting archive
  - Installing maglnet/composer-require-checker (dev-patch-1 57ed1dc): Cloning 57ed1dc74a from cache
5 package suggestions were added by new dependencies, use `composer suggest` to see details.
Generating autoload files
9 packages you are using are looking for funding.
Use the `composer fund` command to find out more!

->

~/dev/test • test/vendor/bin/composer-require-checker
ComposerRequireChecker dev-patch-1@57ed1dc74af28e5613a3f65929b39f3011c2a76b
There were no unknown symbols found.

with

~/dev/test • php -v
PHP 8.0.1 (cli) (built: Jan  8 2021 02:39:34) ( NTS )
Copyright (c) The PHP Group
Zend Engine v4.0.1, Copyright (c) Zend Technologies
    with Zend OPcache v8.0.1, Copyright (c), by Zend Technologies

Looks good to me 👍

Copy link
Collaborator

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚢

@Ocramius Ocramius merged commit 2a3c2de into maglnet:master Jan 18, 2021
@Ocramius
Copy link
Collaborator

Rolling out a release ASAP

@martinssipenko
Copy link
Contributor

@Ocramius will 3.0 phar also be available for download from releases?

@Ocramius
Copy link
Collaborator

No idea if that's automated, sorry: for now I released for the "traditional" approach, which allows most projects to start using the dependency again after upgrading to php 8

@Ocramius
Copy link
Collaborator

Ok, it requires manual steps: https://twitter.com/MaGlNet/status/1351079268831416323

I can't do a release, since my GPG key is not really part of that trust chain.

@martinssipenko
Copy link
Contributor

All previous versions had phar files available, perhaps you could manually add them to release? This project works better if used as phar instead of composer install as it does not need to drag in unnecessary packages that often conflict with project's requirements.

@Ocramius
Copy link
Collaborator

Please re-read what I just wrote, slowly.

@martinssipenko
Copy link
Contributor

Just read your last comment, after posting my previous one. Great! I'll keep an eye on it. Thanks!

@MidnightDesign MidnightDesign deleted the patch-1 branch January 18, 2021 09:12
@maglnet
Copy link
Owner

maglnet commented Jan 18, 2021

@martinssipenko
I just added a manually build phar file to the release:
https://github.com/maglnet/ComposerRequireChecker/releases/tag/3.0.0

@martinssipenko
Copy link
Contributor

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants