On Mon, Dec 4, 2017 at 9:56 PM, Yves-Alexis Perez <cor...@debian.org> wrote: > On Thu, 2017-11-30 at 16:31 +0100, Christian Ehrhardt wrote: >> Pushed it to the same debian-submission-nov2017 branch as before. > > Thanks, > > I don't really have good news though. I took a look and first, it seems that > the git commit entries aren't really good. git log --format=oneline is barely > readable, for example.
Yeah the format was meant for another tool which made debian/changelog entries out of it. I could rewrite them the next time we talk about this topic in probably 6 months :-) > For the commit contents: > > 1aa409a5 (mass plugin enable): NACK again; I won't enable that many stuff (and > especially not in one go) I can understand, this all is just a suggestion. Things came up (way before my time) due to user requests and if I did history research correctly at that point it was decided to enable a few more to not get requests for extra plugins all the time. You are not taking all - that is fine, if you take a few that is good enough. Since I wasn't part of that old enabling activity I wanted to sync with you here and even considered dropping a bunch of them next (post LTS) cycle. Nobody remembers the particular reasons for some of them, so for all that make no sense to you in a hard enough way to not even enable them for "-extra-plugins I'd consider triggering bug reports in Ubuntu if somebody really used them. I'm fine either way - all I request is that we look and discuss about things every half year or so. So far we benefitted both each time we did, so it isn't wasted time. > d83c243b (duplicheck disable): one good reason for the NACK just above: it's > not enabled in Debian, you just enabled it in 1aa409a5 :) That is a bummer, at the time I saw it the first time I thought it was enabled in Debian and since then carried on. /me feels ashamed and obviously drops that part :-) > 766d4f0d (ha disable): not really sure; it's way easier to have a custom > kernel than a custom strongSwan Ok, so you had real cases where you or a Debian user used that? Very interesting POV, I think neither is easier than the other but I see your point. > 85150f06 (kernel-libipsec enable): for reference, this is #739641 and I'm > still not sure I like it. I might pick it but end up disabling it before > release It is disabled by default as Simon already outlined for the reason to be an opt-in. > a587dc11 (TNC move): I might pick this one because TNC is pretty standalone > per-se and it might make sense to split it, but really that's a lot of new > binary packages. I understand the new-queue fight, but it really came handy for users who wanted it standalone. > 7629c11a7 (test-vectors move): NACK, what's the justification? Also it'll need > some breaks/replaces > f9e7f9007 (CCN move): NACK, what's the justification? Also, the break/replace > have the ubuntu version in them, which is wrong for Debian. Sorry for not converting the breaks/replaces for those two to Debian versions properly. @Simon - thanks for helping with arguments! (he is one of the known-to-me Ubuntu Strongswan users). But in this case it really is CCM. Every time I come back to this I hate that I only inherited this delta - too often I don't know and sometimes even fail to find the reasons for them. In this case due to your Nack I once again spent some more time to search for its history. I didn't find any and honestly I don't care anymore - let me drop these two changes on my end (with Ubuntu breaks/replaces :-) ). > 13300d3bf (libstrongswan.install reorder): ACK, I could pick it if there was > an isolated changelog entry with it I'll on rebase let this be a change+CL entry. I'll also reorder the likely to pick changes at the top so you have not CL conflicts. > 4fe0861e2 (kernel-netlink conffiles): NACK, these files are installed only on > Linux and thus the handling is done in debian/rules Ok, it seems the original author didn't think about non-linux in this case. I'll convert it to a d/rules change inside the DEB_HOST_ARCH_OS check > 76535cba5 (libfast disable): Should be ok, if I have an isolated changelog > entry for this I'll reorder and add CL for this as well > 8dbf648b7 (libcharon-standard-plugin): I can understand the rationale (plugins > for common password-based mobile VPN setup), but I don't really like it. I > don't really like adding a new binary package, and the name is definitely not > good. Also, as far as I understand it, the plugins are useful when you're > actually configuring a client/roadwarrior to imitate a mobile client with its > limitations. I don't think it's a good thing to do, I'd prefer simplifying the > secure uses cases, like pubkeys-based ones. <insert the argument by Simon Deziel's former reply here/> > 9b5769771 (changelog update): NACK for obvious reason, I'd need isolated > changes to the changelog (although obviously it's not simple to cherry-pick > them either, I think I prefer it that way). On the rebase I will revise the changes into the format that you need. > Regards, and again thanks for the work you do. I have to thank you way more - for your review, your discussions and your patience with me trying to sync&sort-out this rather old delta. Every cycle we do on this helps me tremendously, and the 30% we take into Debian each time helps both of us. I'll come back with the revised version later today with all we discussed here so far. Thanks, Christian Ehrhardt