Code review comment for ~bryce/ubuntu/+source/php-defaults:merge-v80--exp1-impish

Revision history for this message
Bryce Harrington (bryce) wrote :

On Wed, May 12, 2021 at 05:51:46AM -0000, Christian Ehrhardt  wrote:
> Review: Approve
>
> In Debian "8d1ad3fd d/control: Make php packages cross-installable to i386" contains our Delta.
> It was actually re-fixed better (to not be geenrated away) in "07799e8c6" later.
>
> The changelog is irrelevant as you want to sync it.
>
> I didn't see anything suspicious in the build logs.
>
> Checking the Delta in between for unwanted side-effects
> $ git log -p debian/76..debian/80_exp1
> - dh10 -> ok
> - git branch renaming -> ok
> - adding php 8.0 (as alternative)
> This is ok, but as we know those transition will need a lot of nudging to fully complete
> I guess eventually for 21.10 we want only one PHP, so maybe take a note/card to drop
> 7.4 in time here?

Yep, that's a standard step in the transition process. I don't really
need a discrete card for that, but maybe I should establish a checklist
somewhere more formally than my personal todo's.

> - add xml to php-all-dev
> This will be a component mismatch php-all-dev (main) -> php8.0-xml (universe)
> So far src:php8.0 isn't promoted yet, this will need (a trivial) MIR
> And is another case of "we then need to get rid of 7.4 this cycle"

Yep, that MIR is probably my next task to tackle.

> - our Delta for i386 -> ok
> - start the 8.0 transition properly -> yeah I've seen this coming above, make sense
> This is dropping Ubuntu 12.04 code, which is fine
> -
>
> On thing thou, Debian-git already is preparing v82:
> php-defaults (82) unstable; urgency=medium
> * No change version bump.
> php-defaults (81) unstable; urgency=medium
> * php8.0: XMLRPC has been moved from base to PECL
> * Add phpX.Y-mbstring to php-all-dev dependencies
> * Add virtual php-gettext package
> Making the package will pick those up as they appear, but only if they do so in -unstable.
> Since it is unlikely for some months that it appears in unstable we might need to consider re-merging from git later on this cycle?
>

Yeah I had noticed that as well and mulled over pulling from git
instead. But I figure it's easier to sync experimental 1st and then
sync in more as needed, then to sync git and then back stuff out if it
causes problems. Good advice though to keep an eye on it.

> Due to the above php8.0-mbstring also will need to get to main.

Yep, I'd like to better understand what it is and what the significance
is, but good point this will need mentioned in the MIR.

> I haven't found anything, but to be sure was any of those packages
> explicitly excluded in the past and would become a problem?

Not to my knowledge.

> Overall:
> - it does look all good to me
> - yes we want to get the transition started and this is what it is doing
> - You can
> a) sync it now
> b) or merge v82 from git
> But for both maybe prepare the MIR for src:php8.0

Thanks again for the review, I will proceed with syncing from
experimental and then work on the MIR.

Bryce

« Back to merge proposal