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.

Sign up for GitHub

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

Jump to bottom

add apcu enabled check if apcu extension loaded (#10310) #10311

Merged
merged 4 commits into from
Dec 18, 2022
Merged

add apcu enabled check if apcu extension loaded (#10310) #10311

merged 4 commits into from
Dec 18, 2022

Conversation

aleksejs1
Copy link
Contributor

No description provided.

@derrabus derrabus added the Bug label Dec 15, 2022
@derrabus derrabus added this to the 2.13.5 milestone Dec 15, 2022
@derrabus
Copy link
Member

Can you take a look at the CI failures?

@aleksejs1
Copy link
Contributor Author

aleksejs1 commented Dec 15, 2022 •
edited
Loading

I have no idea why is CI fails... apcu enabled in setup, but looks like apcu_enabled() return false.
If ini_get will return the same, then need some more time for investigation

@greg0ire
Copy link
Member

greg0ire commented Dec 15, 2022 •
edited
Loading

It makes little sense to enable APCu in the CLI. Because of that, there is another setting if you want to do it anyway.

I think you should set that setting in the failing tests, because it does not make much sense to use AcpuAdapter if that cache system is not actually available with the CLI.

@aleksejs1
Copy link
Contributor Author

I think you should set that setting in the failing tests

Sorry, but how to enable apcu-cli in failing tests? Should it done in ci-configs, or in test annotation?

@greg0ire
Copy link
Member

greg0ire commented Dec 15, 2022 •
edited
Loading

That setting is PHP_INI_SYSTEM, so I'm not sure it can be done in test annotations. In fact, I must confess I didn't know this was doable with test annotations, can you link to the relevant PHPUnit docs?

Anyway, I think I would use https://github.com/shivammathur/setup-php#ini-values-optional in this file: https://github.com/doctrine/orm/blob/2.13.x/.github/workflows/continuous-integration.yml

@derrabus derrabus merged commit c23220b into doctrine:2.13.x Dec 18, 2022
derrabus added a commit that referenced this pull request Dec 18, 2022
@derrabus
Merge branch '2.13.x' into 2.14.x
f94cb9a 
* 2.13.x:
  add apcu enabled check if apcu extension loaded (#10310) (#10311)
@aleksejs1 aleksejs1 deleted the apcu-enabled-check branch December 19, 2022 04:05
derrabus added a commit to derrabus/orm that referenced this pull request Dec 19, 2022
@derrabus
Merge branch '2.14.x' into 3.0.x
e801cac 
* 2.14.x:
  Drop forceful loading of annotations (doctrine#10321)
  Document stdClass structures used by CommitOrderCalculator (doctrine#10315)
  Psalm 5.3.0 (doctrine#10317)
  PHPStan 1.9.4 (doctrine#10318)
  add apcu enabled check if apcu extension loaded (doctrine#10310) (doctrine#10311)
  Add TypedFieldMapper for automatic mapping of typed PHP fields to DBAL types (doctrine#10313)
derrabus added a commit to derrabus/orm that referenced this pull request Dec 19, 2022
@derrabus
Merge branch '2.14.x' into 3.0.x
3a2bfce 
* 2.14.x:
  Drop forceful loading of annotations (doctrine#10321)
  Document stdClass structures used by CommitOrderCalculator (doctrine#10315)
  Psalm 5.3.0 (doctrine#10317)
  PHPStan 1.9.4 (doctrine#10318)
  add apcu enabled check if apcu extension loaded (doctrine#10310) (doctrine#10311)
  Add TypedFieldMapper for automatic mapping of typed PHP fields to DBAL types (doctrine#10313)
@mpdude mpdude mentioned this pull request Jan 26, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greg0ire greg0ire greg0ire left review comments

@derrabus derrabus derrabus approved these changes

Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
2.13.5
Development

Successfully merging this pull request may close these issues.
3 participants
@aleksejs1 @derrabus @greg0ire