Code review comment for ~mirespace/ubuntu/+source/freeipmi:merge-freeipmi_1.6.6-4-impish

Revision history for this message
Miriam EspaƱa Acebal (mirespace) wrote :

Hello,

> So this is indeed a sync and I'll sync it after this, but we still can do the
> review to identify things to improve next time.
>
> 1. there was no review slot for anyone else but canonical-server, since
> reviewing consumes those that isn't enough. You'd usually want to also add one
> for canonical-server-packageset-reviewers or canonical-server-core-reviewers

Annotated

> 2. target branch for a merge from Debian is debian/sid and not as done here
> ubuntu/devel
> That also explains the odd visualization in the LP preview
>

Annotated 2

Make a note (for myself in the future): For merge MP, look here https://github.com/canonical/ubuntu-maintainers-handbook/blob/master/PackageMerging.md#submit-merge-proposal not here https://github.com/kstenerud/ubuntu-maintainers-handbook/blob/master/MergeProposal.md#open-a-merge-proposal.

(I always put all the stuff in the Commit message not in the Descrition part and then I'm going to check on UMH what textbox is the correct one before submitting -shame on me-)

> 3. I checked the build in the PPA
> It is sadly only x86 (you'd usually want all arches we build on the real
> upload) and
> without proposed configured (so it built with glibc 2.33 instead of the new
> one).
> The build is good to be done, but the closer you are to the real build on
> an upload/sync the
> better.

Totally forgot to activate the other builds in the PPA. Indeed, I waste a lot of time because I was trying to upload the debian upstream package to the PPA... until I realized I can't do that because I can't sign that :facepalm:)

> The changelog is a common place for mistakes, but I must say if we would
> upload (and not sync) this it LGTM - thanks!
>

:) Thanks a lot! As it was my common place to fail, I took great care of it... now I will do in the others aspects also (I hope so)!

> +1 to sync this

« Back to merge proposal