Thank you, Lucas. Here's my review. I focused on the Debian packaging stuff, but also tried to review the overall changes to the source code. Also, some of the points I raise below can be considered nitpicky/wishlist. ## dep8 tests Before I forget: I triggered the dep8 tests for Mantic. Here's the result: Results: (from http://autopkgtest.ubuntu.com/results/autopkgtest-mantic-ua-client-staging/?format=plain) ubuntu-advantage-tools @ amd64: 02.08.23 19:27:59 Log 🗒️ ✅ Triggers: ubuntu-advantage-tools/29~rc1 ubuntu-advantage-tools @ arm64: 02.08.23 19:48:24 Log 🗒️ ✅ Triggers: ubuntu-advantage-tools/29~rc1 ubuntu-advantage-tools @ armhf: 02.08.23 19:23:09 Log 🗒️ ✅ Triggers: ubuntu-advantage-tools/29~rc1 ubuntu-advantage-tools @ ppc64el: 02.08.23 20:17:12 Log 🗒️ ✅ Triggers: ubuntu-advantage-tools/29~rc1 ubuntu-advantage-tools @ s390x: 02.08.23 19:36:43 Log 🗒️ ✅ Triggers: ubuntu-advantage-tools/29~rc1 ## lintian messages I noticed that you do read the lintian messages (good!) and take care to handle them when needed, so I decided to comment on some of the existing messages that I believe could be addressed as well. - maintainer-script-calls-systemctl: you should be able to use "deb-systemd-invoke" instead of invoking "systemctl" directly in the postinst script, because "deb-systemd-invoke" will simply execute "systemctl" with whatever arguments you pass to it. - possible-bashism-in-maintainer-script: you can safely override this one. - command-with-path-in-maintainer-script: I see that this has been done this way because of LP #1930121, so it's OK to override this (and leave a comment referencing the bug). - package-supports-alternative-init-but-no-init.d-script: you can safely override this one (Ubuntu doesn't support sysv init.d). - systemd-service-file-missing-documentation-key: I'd recommend adding the Documentation directive in the systemd service files. There are two override flags that can probably be dropped (invalid-arch-string-in-source-relation and missing-build-dependency-for-dh_-command) because lintian is saying that they're unused. ## d/changelog - The new README file is called d/README.source (you missed the ".source" suffix). - Typo: subsctiption - The text after "- proxy:" starts with a capital "A". It should follow the convention and start with lowercase "a". - Nitpick: you might also mention the d/copyright update here, just for completeness sake. No need to rebase the branch just for that, though. I still have to look deeper into the contents of d/changelog vs. the commits. ## d/README.source LGTM, but I have a question: - According to the discussion on https://github.com/canonical/ubuntu-pro-client/issues/2463, you had decided to keep the documentation bits as part of the tarball but mention that they're not actually used to generate the binary package. However, df505d4643a20a7210a9c9ae972678549adc0801 seems to indicate that you decided to move the documentation out of the tarball. Is that correct? Is this why I don't see a mention to the documentation part in the "Particularities" section here? ## d/*postinst - Nitpick: you could have coded "rename_gpg_keys" using a for loop. I believe it would be easier to review/catch mistakes. ## preferences.d/ubuntu-pro-esm-* - Nitpick: can I suggest that you add an empty line between the first the second lines? Also, it's good practice to write entries using the imperative mood (like you're already doing in the d/changelog entry). Here's my suggestion: >>>>> # This file was created by ubuntu-advantage-tools # Pin esm-... packages to a slightly higher value than the archive # so those are preferred when the service is enabled. Package: * ... >>>>>> ## General questions - When I run "git log pkg/ubuntu/devel..upload-29-mantic", I see that at least commit b33fb5e3 (lock: alert the user about corrupted lock) seems to have been released already. Maybe I'm doing something wrong here, but could you explain to me why this is happening please?