Skip to content
This repository was archived by the owner on Nov 19, 2024. It is now read-only.

Update PHP settings on performance best practices software page #8396

Conversation

chiranjeevi-cj
Copy link
Contributor

Purpose of this pull request

This pull request (PR) Updates PHP settings on the performance best practices software page

Affected DevDocs pages

Fixes #8394

Copy link
Contributor

@BarnyShergold BarnyShergold left a comment

Choose a reason for hiding this comment

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

Can the second reviewer double-check the technical truthfulness of this addition? I'm not aware that this setting has anything to do with updating env.php

Co-authored-by: Barny Shergold <barny.shergold@vaimo.com>
Copy link
Contributor

@BarnyShergold BarnyShergold left a comment

Choose a reason for hiding this comment

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

@hguthrie - Could do with a second opinion on the technical accuracy of this change. 'm not aware of a PHP setting having this effect

@hguthrie
Copy link
Contributor

hguthrie commented Dec 21, 2020

It may seem that adding this note will help, but after some research, I found that the next release of Cloud tools addresses this with a less manual approach. See #8240
@bdenham For this issue, can you review whether the release note is sufficient or if anything further added to the Cloud guide, or other guide would be helpful?

Copy link
Contributor

@hguthrie hguthrie left a comment

Choose a reason for hiding this comment

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

This may not be necessary. Waiting for feedback related to release notes.

@hguthrie hguthrie added the Waiting for Response Waiting for response from internal/external parties label Dec 21, 2020
@hguthrie hguthrie assigned bdenham and unassigned hguthrie Jan 5, 2021
@bdenham
Copy link
Contributor

bdenham commented Jan 12, 2021

Sorry I'm late to the party. Looking at this now and finding a second opinion about the accuracy. It is more useful than the release note because of the details it provides.

@bdenham
Copy link
Contributor

bdenham commented Jan 12, 2021

I talked to @meker12 about this and as she said: " For Magento on-premise, the default for opcache is 1 and the core documentation is correct. This note might need to go in the Cloud documentation, because the people that are going to run into the problem are Cloud users."

Margaret also mentioned a support article for the issue: https://support.magento.com/hc/en-us/articles/360050422532

I'm still waiting to hear back from a Cloud developer, but at a minimum, it looks like this needs to be added to the Cloud Guide, not Performance Best Practices.

@meker12
Copy link
Contributor

meker12 commented Jan 12, 2021

I talked to @meker12 about this and as she said: " For Magento on-premise, the default for opcache is 1 and the core documentation is correct. This note might need to go in the Cloud documentation, because the people that are going to run into the problem are Cloud users."

Margaret also mentioned a support article for the issue: https://support.magento.com/hc/en-us/articles/360050422532

I'm still waiting to hear back from a Cloud developer, but at a minimum, it looks like this needs to be added to the Cloud Guide, not Performance Best Practices.

@hguthrie mentioned the 2.1.4 ECE tools improvement that sets the opcache_cli to the correct setting for Cloud by default (see #8240) . That's what I thought -- Cloud sets this to the right setting by default, so if people run into issues, then the support article is there. (Assumes that Cloud project uses ECE-tools 2.1.4 or later.)

@dobooth
Copy link
Contributor

dobooth commented Jan 19, 2021

@bdenham @hguthrie any updates here? Ready to go?

@hguthrie
Copy link
Contributor

@dobooth This has a support article that provides this information in further detail. The next release of Cloud tools contains corrections or additional information for this issue. If we add this note now, it has to be cleaned or updated for the Cloud release. @bdenham stated he is waiting for Cloud team feedback, so I will mark this PR as waiting. If @bdenham decides to proceed with this PR, then this change must be reviewed again during the Cloud docs release so that it agrees with the changes made by the Cloud team.

@dobooth dobooth marked this pull request as draft January 27, 2021 13:32
@bdenham
Copy link
Contributor

bdenham commented Feb 4, 2021

Yeah, I'm going to close this for all the reasons mentioned above. The setting is now set correctly by default, so it would create more confusion if this was added. Regardless, thanks @chiranjeevi-cj for your heads-up work on this.

@bdenham bdenham closed this Feb 4, 2021
@ghost
Copy link

ghost commented Feb 4, 2021

Hi @chiranjeevi-cj, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@hguthrie hguthrie removed the Waiting for Response Waiting for response from internal/external parties label Feb 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Partner: EY partners-contribution PR created by Magento partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update PHP settings on performance best practices software page
7 participants