Problem/Motivation

#2388255: (followup) Limit PDO MySQL to executing single statements if PHP supports it introduces an REQUIREMENTS_INFO for the recommended PHP version.

Proposed resolution

Change it to REQUIREMENTS_WARNING and fix the tests.

I'm not sure if this requirement warning is important enough to show during install anyway though (maybe only after install). It would be an annoying interruption for anyone installing with PHP 5.4 since it's not too likely they'll drop what they're doing and switch to a different PHP version in the middle of the install.

Discuss this.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#6 make_php_version_info_a-2452587-6.patch1.36 KBhussainweb

Comments

dawehner’s picture

dawehner
Primary language German
commented
Issue summary: View changes
timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
commented
Priority: Normal » Major
Status: Postponed » Needs work

In #2524432: Suggest PHP 5.6 as the recommended version I am working to modify the work done in #2388255: (followup) Limit PDO MySQL to executing single statements if PHP supports it and also add a recommendation for PHP 5.6.5.

I don't believe REQUIREMENT_INFO give enough notice to the potential issue. REQUIREMENT_INFO is used for the standard PHP version information, therefore stating "everything is ok". I feel we need to use REQUIREMENT_WARNING more.

Also why are we using REQUIREMENT_INFO when we should be using REQUIREMENT_OK?

I am reopening this and specifically for the REQUIREMENT_INFO vs REQUIREMENT_OK aspect I'm making this "major"!

daffie’s picture

daffie commented

+1 or changing the REQUIREMENT_INFO to REQUIREMENT_WARNING for sites using a PHP version lower then 5.6.5.

dawehner’s picture

dawehner
Primary language German
commented

+1 or changing the REQUIREMENT_INFO to REQUIREMENT_WARNING for sites using a PHP version lower then 5.6.5.

I just don't believe what should block people from using 5.5, its perfectly fine IMHO, so INFO for the general works. I much rather care about the subversion thing.

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
commented

So... I've been doing some digging.

I think we should use REQUIREMENT_INFO for the recommended PHP info, it's not a blocker so not a warning, it's not really perfect so not ok, therefore it's info. However the issue is Drupal 8 default all requirements that don't have an explicit severity set default to info, I believe they should default to ok, then we can bubble up things like PHP version as info.

The new patches in #2528242: Add color-* classes to all rows on the status page and #2528270: Use RequirementSeverity::OK by default help to move this forward.

hussainweb’s picture

hussainweb
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new make_php_version_info_a-2452587-6.patch1.36 KB

I am just posting a patch to get things started. I am using REQUIREMENT_INFO as that seems to be the consensus as of #5.

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
commented
Status: Needs review » Closed (won't fix)

@hussainweb - This patch looks to duplicate the work in #2524432: Suggest PHP 5.6 as the recommended version.

Marking as won't fix because #2524432: Suggest PHP 5.6 as the recommended version will cover adding a check for 5.6.5, then #2528242: Add color-* classes to all rows on the status page and #2528270: Use RequirementSeverity::OK by default will raise the importance of REQUIREMENT_INFO.

hussainweb’s picture

hussainweb
Primary language English
commented

Funny, policy issues usually don't have patches.