Hi Heather, Some notes:
In the [Testcase] section, it is useful to link the ppa you build your packages in, e.g. https://launchpad.net/~hypothetical-lemon/+archive/ubuntu/lp2110521-sssd-port/ This makes it easier for affected users to find test packages, and maybe even help test them. It also lets others review what debdiffs your built packages have. For versions of your test packages, I always recommend NOT using versions that would be present in the archive. This creates confusion over what is and is not a test package, and creates confusion for customers if they are running official packages or test packages. I recommend version schemes like: Take the current package version: 2.10.1-2ubuntu4, then: sssd - 2.10.1-2ubuntu4+<case number or lp bug number>vDATEb1 e.g. sssd - 2.10.1-2ubuntu4+sf410625v20250805b1 or sssd - 2.10.1-2ubuntu4+lp2110521v20250805b1 If you need to make a new build on the same day, increment b1 to b2 and so on. This way, you know exactly if this is a test package or not, what case / bug it was for, and what version it is, so when it turns up in a sosreport after running in production for two years, we know exactly what it is, even after the ppa is long gone. There is also a concept of "burning versions" where once a version is uploaded to a ppa or the archive, the version cannot be used again, and must be incremented. Using the test package naming helps avoid ever accidentally burning a version in the official ubuntu archive. Your [Where problems can occur] section needs work. I recommend reading https://documentation.ubuntu.com/sru/en/latest/reference/bug-template/ for the general gist of things. The SRU team really want you to consider that any change always implies a risk of things going wrong. Where could problems happen? For you, this would likely be around parsing and using certificates in smartcards, causing authentication issues. What would a regression look like to a user? How would they workaround it? In the [Other info] section, I usually like spelling out what commits you are actually pursuing for this SRU. You linked the commit that solves the problem, yes, https://github.com/SSSD/sssd/commit/782a6dd54967e7c2dd3013f7e68134ee8751ab88, but when I open your patches, I see four commits listed. I try and include a list of commits I will be including, as well as what upstream version the commits landed in. For your patches themselves, your filenames are: d/p/0001-P11_CHILD-Invert-if-statement-to-reduce-code-nesting.patch d/p/0002-P11_CHILD-Implement-passing-const-args-to-get_pkcs11.patch d/p/0003-P11_CHILD-Extract-slot-processing-into-separate-func.patch d/p/0004-P11_CHILD-Make-p11_child-iterate-over-all-slots.patch I really would like to see what LP bug these are attributed to, e.g. lp2110521-1-P11_CHILD-Invert-if-statement-to-reduce-code-nesting.patch lp2110521-2-P11_CHILD-Implement-passing-const-args-to-get_pkcs11.patch lp2110521-3-P11_CHILD-Extract-slot-processing-into-separate-func.patch lp2110521-4-P11_CHILD-Make-p11_child-iterate-over-all-slots.patch These filenames are also quite long, I would recommend trimming them down to 70 characters. lp2110521-1-P11_CHILD-Invert-if-statement-to-reduce-code-nestin.patch lp2110521-2-P11_CHILD-Implement-passing-const-args-to-get_pkcs1.patch lp2110521-3-P11_CHILD-Extract-slot-processing-into-separate-fun.patch lp2110521-4-P11_CHILD-Make-p11_child-iterate-over-all-slots.patch etc. Your dep3 tags are good, but you could also add the upstream bug as well. e.g. Origin: upstream, https://github.com/SSSD/sssd/commit/3392a857c76ccb75e487ec122121cb5e071bff49 Bug: https://github.com/SSSD/sssd/issues/5905 Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/sssd/+bug/2110521 Last-Update: 2025-07-16 Did any of these commits need backporting? Under "origin", "upstream" is for cherry picks, while "backport" is used for patches where you needed to adjust context, backport the code etc. Now, I usually like tidying the headers up, e.g. commit 3392a857c76ccb75e487ec122121cb5e071bff49 From: Georgij Krajnyukov <[email protected]> Date: Wed, 19 Feb 2025 11:59:14 +0300 Subject: P11_CHILD: Invert if statement to reduce code nesting Reviewed-by: Alexey Tikhonov <[email protected]> Reviewed-by: Sumit Bose <[email protected]> Origin: upstream, https://github.com/SSSD/sssd/commit/3392a857c76ccb75e487ec122121cb5e071bff49 Bug: https://github.com/SSSD/sssd/issues/5905 Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/sssd/+bug/2110521 Last-Update: 2025-07-16 It looks a bit nicer than raw git headers, and it is also dep3 compatible with the Subject field. Lastly, you can also do a "quilt refresh" on your patches so they match the source code, and no ambiguous fuzz needs to happen in the future. Note it does change from git to the quilt format, but thats okay. If you upload multiple patches for a single release to a bug, e.g. you needed to fix something in a patch for noble, delete the old patch and just upload the new one, to reduce confusion over what needs to be sponsored. If you need any inspiration for SRU templates, have a look any of my existing LP bugs for ideas on formatting. I will re-review once you update the SRU template and your debdiffs. Let me know if you have any questions. Thanks, Matthew -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2110521 Title: Continue searching other PKCS#11 tokens if certificates are not found To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/sssd/+bug/2110521/+subscriptions -- ubuntu-bugs mailing list [email protected] https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
