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

fix: Inconsistent directives value between Default and OPCache groups #9476

Merged
merged 3 commits into from
Mar 19, 2025

Conversation

warcooft
Copy link
Contributor

@warcooft warcooft commented Mar 5, 2025

Description
Explain what you have changed, and why.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@paulbalandan
Copy link
Member

I agree to the recommended value, but why suggesting to enable on CLI?

From ChatGPT: https://chatgpt.com/share/67d9b778-26d4-800f-9a81-f1cdaf6f7581

@warcooft
Copy link
Contributor Author

warcooft commented Mar 19, 2025

This choice actually depends on the user's needs. As GPT mentioned, If you often run queues with cron jobs like send broadcast emails, database migration and seeding, or repetitive CLI scripts, enabling OPcache can be a good choice because it reduces parsing and compilation overhead. But if there is no intense activity in CLI mode, it is better to leave it disabled to save resources.

I think we need to improve the message to make it easier to understand,

E.g. Enable if you're running queues or repetitive CLI tasks

@paulbalandan
Copy link
Member

Yes, I think we need to put that instead.

Co-authored-by: John Paul E. Balandan, CPA <[email protected]>
@paulbalandan paulbalandan requested review from michalsn and ddevsr March 19, 2025 12:37
@paulbalandan paulbalandan merged commit c5e1c8b into codeigniter4:develop Mar 19, 2025
49 checks passed
@warcooft warcooft deleted the command-phpini branch March 19, 2025 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants