I've completed the review of the rest of the adsys delta. This looks
good to me; my only observation is that as part of the change of i18n
handling libraries, there are two regressions in the localizability of
strings:
internal/adsysservice/service.go: updateFmt := "%s" + gotext.Get(", updated on
") + "%s"
internal/cmdhandler/suggest.go: requireMsg := "%s " + gotext.Get("requires a
valid subcommand")
Both of these introduce assumptions about the word order of the
sentences which were not present before, making the new version less
translatable than the previous version.
However, I will not block the SRU on this because I see that the only
language adsys is currently translated to is French, which is not
affected. Still, it is bad form to use concatenation of translated
strings like this, which you appear to know because both of these are
marked with 'fixme's in the code.
For the other issues with the packaging:
> - debian/prerm: thank you for catching this, I don't know how it slipped
> through, I guess it was all the false encouragement given by packages in the
> wild that employ the same (broken) prerm workflow. I've opened a PR to revert
> this upstream https://github.com/ubuntu/adsys/pull/1026 -- I don't think it
> adds much value to have this working (and it was not explicitly documented in
> the changelog either, so I'm happy with removing the broken code, as it never
> worked)
Yes, please revert this, thanks.
> - krb5/winbind as an alternative to sssd: this was explicitly requested by
> paying customers with specific requirements related to cross-domain forest
> setups which could not be satisfied by sssd directly -- it's documented
> (albeit
> in a terse manner, only mentioning winbind) in the 0.10.0 changelog
Ok, understood (and accepted for SRU).
> - addition of Depends: apparmor: opted for this because the apparmor policy
> manager subprocesses "apparmor_parser" (ref:
> https://github.com/ubuntu/adsys/blob/b20890f78bd55d5b5c90d77af61d17eb31f40b77/internal/policies/apparmor/apparmor.go#L82)
> -- happy to drop it if you think it's the best practice
Since you are invoking programs from this package directly, it's fine to
leave this as-is.
> - additional of dependency on nfs-common: opted for a "Depends" here because
> adsys could apply a nfs mount policy which would fail hard if nfs-common were
> not installed -- I would've gone for "Recommends" in case of a soft warning
Ok, please leave this as-is.
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/2059756
Title:
[SRU] adsys 0.14.1
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/adsys/+bug/2059756/+subscriptions
--
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
