View in #core on Slack
@Matt_Robinson: It is time for the weekly PR meeting, I’m running behind and will join shortly, anyone online feel free to start without me.
@Simone_Fioravanti - I saw your testing on 902 - are we happy to merge now?
@Simone_Fioravanti: Happy!
@Matt_Robinson: Thanks, that is merged.
Laurence has been looking at 815, something we need to get working and merged for future PHP 8.x testing.
In the PR all looks good but he wan into some issues:
https://github.com/bahiirwa/ClassicPress/pull/2
I think he has tried merging the PHPUnit tests over a backport of a changeset for MySQL 8:
https://core.trac.wordpress.org/changeset/53897
Hmm, looks that way - it might have been cleaner to test the other way around and merge 53897 changes over the PHPUnit update.
Anyone any thoughts here?
@Joy_Reynolds: The PR is a merge of two changes?
@Matt_Robinson: Laurences? It’s a merge of 2 changes, one being a single changesets backport and one PR being a significant set of changesets and commits.
Perhaps if we just focus on the core PR and we can look at 53897 later when needed - there are more testing change needed I’ve been working n that will allow PHP 8 testing but also testing on MySQL 8 in time too.
@Joy_Reynolds: What did you decide to do with the PHPUnit changes that WP did?
Since we don’t need all the older versions to test with, we only need part of that mess?
@Matt_Robinson: We can’t fully follow their lead as they rely heavily on pre-built docker containers with PHP, MySQL an WP versions all bundled together.
We are still testing PHP 5.6 upwards and for the time being we can leave that alone.
What we need to add is PHP 8 and 8.1 so we can investigate broken and failing tests.
@Joy_Reynolds: They made a lot of changes for snake case and camel case, so that multiple versions of PHPUnit could be used, but after that I didn’t follow it.
@Matt_Robinson: Do you mean things like setUp()
to set_up()
? That all enables PHP backwards and forwards compatible polyfills for different versions of PHPUnit as I understand it and that is in the PHPUnit 7 & 8 PR.
@Joy_Reynolds: Yes, that. I saw the changes and didn’t follow the details.
I think you need the PHPUnit stuff first.
@Matt_Robinson: Are we happy to merge #815 today based on the above then?
I’ll merge #815 shortly.
Joy, while you are here I had a look at your body only theme PR. Do you think we can include a theme that takes advantage of the change and perhaps also make us of it in further testing? I think you mentioned that you’d edited a previous core theme accordingly?
@Joy_Reynolds: I’m not sure what you are asking. Unit tests? or bundled theme? or something else?
@Matt_Robinson: Both, if we can bundle a theme we should also be able to add further unit tests that activate that bundled theme and perhaps ensure that the output is as expected.
@Joy_Reynolds: That is integration testing, not unit testing.
@Matt_Robinson: I was looking at the theme testing as it stands in the units tests but can’t remember now what I found.
@Joy_Reynolds: I thought a bundled theme using it would be optimal. But the progress here is glacial.
@Simone_Fioravanti: Maybe merging the PR will speed up?
@Joy_Reynolds: Maybe I missed a few unit tests, like for the new functions that output the <head>
section.
@Matt_Robinson: Perhaps if we bundled a theme then, at least then the new functions can be end user tested. If you have time to add some tests for the output of the newly introduced functions that’s be great.
Thinking about it now I don’t think there were any tests then checked full theme output - that would be a bit crazy to manage long term.
@Joy_Reynolds: The tests I wrote are for the new attribute function. Once that function is in, other core functions can be changed to use it, and the existing tests might be affected…but it’s just a filter, so maybe not.
But the body-only stuff can only really be tested with a theme.
@Matt_Robinson: Can you add a theme to current PR?
@Joy_Reynolds: Do you mean like a zip file attached to test with?
or a bundled theme?
@Matt_Robinson: We can do that, or a zip share here , or mabye better yer a basic theme set of files included in wp-content
of the branch that created the PR,
@Joy_Reynolds: that would make it bundled, wouldn’t it?
@Matt_Robinson: If added to the PR and merged it will be bundled with future downloads I think - yes.
@Joy_Reynolds: I think that would be a separate PR.
@Matt_Robinson: Depends if you are willing to share your efforts that way - if not maybe better shared here for prior to release testing.
@Joy_Reynolds: I don’t mind making a theme to bundle, but I don’t think it belongs on that same PR.
@Matt_Robinson: Whichever way you prefer.
Anything else quick, my time is almost up for this week.
Awesome - thanks Joy. Until next week - and it looks like the PHPUnit changes merge okay too!