View in #core on Slack
@Matt_Robinson: It is time for the weekly PR review and issue discussion, who is online and available?
We have managed to merge a few PRs over the last week removing missed Block Editor code snippets and also updating dependencies.
I know there has been some debate about the “Core Plugins” proposal - @Simone_Fioravanti - was the original idea here to just ensure that the CP Plugin directory plugin is loaded early?
@Simone_Fioravanti: Really don’t think that the CP Plugin directory plugin needs to load early. I was thinking at core plugins and made the PR as a proposal.
@Matt_Robinson: Is there still a priority for Core plugins? I did some work about three years ago but it never progressed further. It will make it harder to keep CP in sync with WP if that is a future goal.
Maybe @Viktor and the directors are better positioned to answer the question about priorities.
@Simone_Fioravanti: We are working on WP compatibility, but don’t know how much we can do with polyfills, but with some plugins compatibility means loading a lot of JS, and that can be Core Plugins territory.
@Viktor: It’s not a huge priority. It came up while Simone was working on directory integration.
@Matt_Robinson: So is there an issue with the current core code and directory integration?
@Viktor: So far, the work Simone and I did is a regular plugin.
It’s a PoC for now.
@Matt_Robinson: Got it, if it is a PoC then for now can we continue to discuss and consider - no need to rush to finish a solution and merge when it might not be quite what we need - have I got that about right?
@Simone_Fioravanti: Yes, agree. Maybe label the PR as status:discussion?
@Viktor: I guess right now it can be called “feature plugin”, what WP does while developing core features.
@Simone_Fioravanti: https://github.com/ClassicPress/ClassicPress-v2/pull/51 is needed for Directory plugin to work.
@Viktor: I think we can put the PR on hold until we get somewhere closer to public beta for v2.0.
@ElisabettaC77: Core plugins as it was intended was to move features out of core (like emojis) and as WP works now it is IMHO not feasible anymore without breaking away completely. It’s something we can however revisit when CP is mature enough to abandon WP and go it’s way.
@Matt_Robinson: #51 looks pretty good now to merge, I guess the json endpoints are just to make sure we don’t run a test on something that changes in the future. But other than that the change looks pretty simple.
@Viktor: Right now the endpoints are not functional. Will that cause any problems?
@Matt_Robinson: I don’ believe so, my concern was we add a fixed endpoint in Unit Tests and at some point in the future we accidentally authorise a plugin at the same endpoint and create some very weird behaviour in our tests.
@Simone_Fioravanti: The endpoints are not called. It’s just to ensure that a plugin without Update URI but with Requires CP will use the directory for updates.
Those endpoints can return whatever without affecting tests.
@Matt_Robinson: So for me, if the endpoint doesn’t’ currently work, it is fine, and we just need to remember to block that as a possible endpoint when they go live. So, for me #51 can merge.
@Simone_Fioravanti: For me is ok. And even if a plugin will be called “Unit Test 1” nothing will break.
@Matt_Robinson: Simone - this is your PR, would you like to merge?
@Simone_Fioravanti: Sure, thank you 
Add fallback Update URI when parsing plugin data is a good commit name?
@Matt_Robinson: #25 is pretty much ready to merge too, I’ve been thinking about use_block_editor_for_post and use_block_editor_for_post_type as I’ve left those in ClassicPress in #47 but made them return false. These functions and the filter can be used to work correctly in ClassicPress so I’m thinking we just leave them out of the scope of #25, at least for now.
@Simone_Fioravanti: Agree, we can always add other polyfills later.
@Viktor: I support your decision.
@Matt_Robinson: Simone - would you like to merge #25 too?
@Simone_Fioravanti: Sure!
@Matt_Robinson: There was discussion on #47 about those 2 functions in relation to our Wp-compat approach too, are we happy to just leave those functions returning false now? Any plugin calling them should run it’s Classic code or bail out if running the check correctly.
@Simone_Fioravanti: Agree to leave them just returning false.
@Matt_Robinson: Okay, so we can merge #47 and #50 now too.
I can then grep the code again for any further block code, it’s harder when multiple PRs already exist.
@Simone_Fioravanti: Sure… I’ve seen something in deprecated.php . Then you can close issue #40.
@Matt_Robinson: #50 needs a rebase, I’ll sort that and merge later.
@Simone_Fioravanti - have you got any further with the Timezone error message you are seeing that I seem unable to re-produce?
@Simone_Fioravanti: No, I’ll continue digging into it. Don’t know if just fix without understanding what is wrong can be a solution.
@Matt_Robinson: Agreed, we need to try and figure out why that happens for you. @Viktor or @ElisabettaC77 have you tried reproducing it? Details here:
https://github.com/ClassicPress/ClassicPress-v2/issues/11
@Simone_Fioravanti: Is there a way to have nightly builds working (also integrated with our API)? This way we can test 2.0 broadly.
@ElisabettaC77: I use Rome as my setting and that never occurred. Seems to me that CP PHP clashes with server PHP? Like server on 7.4 while CP expecting 8.x? I use CP on PHP 8 and this error never pops out. And I do live in Italy and connect to the site from Italy
And I have several sites I am building with CP for my portfolio…
@Simone_Fioravanti: You need to have deprecations logging on. It happens from PHP 8 to 8.2.
@Matt_Robinson: I did look at nightly builds but it was complicated. The nightly repository contains the built code as source too so we’d end up over-writing v1 and v2 repeatedly.
The script used is open source here:
https://github.com/ClassicPress/nightly-builder
If anyone can figure out a simple way that works…
@ElisabettaC77: The third param (stringArray $subject) should get “Rome” as a value instead is left empty for some reason.
@Simone_Fioravanti: I’ll take a look at the script.
@ElisabettaC77: Let me test something for the timezones issue. Will report if I get to the bottom of it
@Viktor: Sorry, I haven’t tried it yet.
@Matt_Robinson: Anyone have anything else for today?
@Viktor: I’m working on the URLs, hopefully by next week will have a list of them completed.
@Simone_Fioravanti: For me, that’s all.
@Matt_Robinson: Thanks everyone, until next week then.