Skip to content

Conversation

@TimWolla
Copy link
Owner

No description provided.

@zzjin
Copy link

zzjin commented Feb 14, 2021

need any update to merge?

@TimWolla
Copy link
Owner Author

PHP 8 does not bring any benefit for this Docker image, because it is not meant to run custom software. It however possibly introduces breakage. Thus I plan on sitting on this PR for a little longer to make sure any PHP bugs and Adminer incompatibilities are fixed.

@TimWolla TimWolla marked this pull request as ready for review August 29, 2021 13:51
@TimWolla TimWolla merged commit c67535d into master Sep 2, 2021
@TimWolla TimWolla deleted the php8 branch September 2, 2021 10:17
TimWolla added a commit to TimWolla/official-images that referenced this pull request Sep 2, 2021
@guilliamxavier
Copy link

It seems that Adminer is still not fully compatible with PHP 8, see e.g. vrana/adminer#429, I also have a regression where tables status is all empty cells...

@TimWolla
Copy link
Owner Author

TimWolla commented Oct 6, 2021

@guilliamxavier There's also #108. I was hoping to sit that out, because at least #108 is a not-as-severe issue. But Adminer returning incorrect or missing information is bad of course. I'll try to get a revert done this evening.

@guilliamxavier
Copy link

My bad, the "tables status is all empty cells" regression is not because of PHP 8 but Adminer 4.8.1, as downgrading to 4.8.0 fixes it 😖

@guilliamxavier
Copy link

Wait... I inspected https://github.com/vrana/adminer/compare/v4.8.0...v4.8.1?w=1 but didn't see what could have caused the regression, then I realized that the Docker image adminer:4.8.0 was based on php:7.4 🤦‍♂️ (to be sure of the cause, one should try Adminer 4.8.0 on PHP 8.0 and/or Adminer 4.8.1 on PHP 7.4...)

@TimWolla
Copy link
Owner Author

TimWolla commented Oct 8, 2021

#110

@guilliamxavier
Copy link

Thanks for the revert. After a docker pull adminer I now have Adminer 4.8.1 on PHP 7.4.24 and I confirm that the regression is gone 🙂 (now I would like to report the bug to Adminer but their GitHub doesn't accept issues, requires a SourceForge account...)

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.

3 participants