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

Add json to core extensions and minor improvements #351

Merged
merged 1 commit into from
Jul 6, 2022

Conversation

vjik
Copy link
Contributor

@vjik vjik commented Jul 6, 2022

Fix #350

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.

Approach looks good - ext-json is indeed always baked in, unless someone really wants to disable ext-json during PHP compilation

@Ocramius Ocramius added this to the 4.1.0 milestone Jul 6, 2022
@Ocramius
Copy link
Collaborator

Ocramius commented Jul 6, 2022

Urgh, this again:

  infection/extension-installer contains a Composer plugin which is blocked b  
  y your allow-plugins config. You may add it to the list if you consider it   
  safe.                                                                        
  You can run "composer config --no-plugins allow-plugins.infection/extension  
  -installer [true|false]" to enable it (true) or disable it explicitly and s  
  uppress this exception (false)                                               
  See https://getcomposer.org/allow-plugins

Can you run a local composer install and adjust the plugins section, perhaps?

@vjik
Copy link
Contributor Author

vjik commented Jul 6, 2022

Can you run a local composer install and adjust the plugins section, perhaps?

infection/extension-installer added to composer.json.

@vjik
Copy link
Contributor Author

vjik commented Jul 6, 2022

Approach looks good - ext-json is indeed always baked in, unless someone really wants to disable ext-json during PHP compilation

Seems, we don't can disable json: https://www.php.net/manual/en/configure.about.php

@Ocramius
Copy link
Collaborator

Ocramius commented Jul 6, 2022

@vjik could you squash out all fixup commits that aren't explicative?

@vjik
Copy link
Contributor Author

vjik commented Jul 6, 2022

@vjik could you squash out all fixup commits that aren't explicative?

That's all. All problems fixed and CI actions pass.

@Ocramius
Copy link
Collaborator

Ocramius commented Jul 6, 2022

@vjik no, I am actively asking to squash bf033bf and 537c8c1 :D

@vjik
Copy link
Contributor Author

vjik commented Jul 6, 2022

@vjik no, I am actively asking to squash bf033bf and 537c8c1 :D

Done. But why? :)

@Ocramius
Copy link
Collaborator

Ocramius commented Jul 6, 2022

Done.

Ah, you didn't eliminate them :D

But why? :)

Because git history is not a dumping ground ;-)

Anyway, I suggest that only 9eb100b survives

@vjik
Copy link
Contributor Author

vjik commented Jul 6, 2022

Because git history is not a dumping ground ;-)

Yes, it's right. I'm usualy use "Squash and merge" in PR:

image

@Ocramius Ocramius self-assigned this Jul 6, 2022
@Ocramius Ocramius merged commit 4d18781 into maglnet:4.1.x Jul 6, 2022
@Ocramius
Copy link
Collaborator

Ocramius commented Jul 6, 2022

Thanks @vjik!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove ext-json requirement for PHP 8+
2 participants