On 23-12-16 22:10, Selva Nair wrote:
>
> On Fri, Dec 23, 2016 at 2:39 PM, Steffan Karger <stef...@karger.me
> <mailto:stef...@karger.me>> wrote:
>
> > On 21/12/16 23:03, Steffan Karger wrote:
> >> On 21 December 2016 at 22:09, David Sommerseth
> >> <open...@sf.lists.topphemmelig.net
> <mailto:open...@sf.lists.topphemmelig.net>> wrote:
> >>> On 18/12/16 19:26, Steffan Karger wrote:
> >>>> Now that we have touched each and every file anyway, I decided
> to go over
> >>>> the code I regularly work with and reformat it some more by
> hand. This is
> >>>> how for I got today, and is a large enough patch I think.
> >>>>
> >>>> This commit is mostly just reordering and changing whitespace,
> with one
> >>>> exception: it removes the #if 0 around some debugging code in
> >>>> read_key_file(), and now always print the debugging if
> D_SHOW_KEYS is
> >>>> enabled.
> >>>>
> >>>> This patch is best reviewed with something like
> >>>> 'git diff --ignore-space-change'.
> >>>>
> >>>> Signed-off-by: Steffan Karger <stef...@karger.me
> <mailto:stef...@karger.me>>
> >>>> ---
> >>>> v2: fix wrong indent, add more 'for () {' -> 'for ()\n{' fixes.
> >>>>
> >>>> src/openvpn/crypto.c | 425
> ++++++++++++++++++++++---------------------
> >>>> src/openvpn/crypto.h | 27 ++-
> >>>> src/openvpn/crypto_mbedtls.c | 63 ++++---
> >>>> src/openvpn/crypto_openssl.c | 38 ++--
> >>>> src/openvpn/cryptoapi.c | 101 ++++++----
> >>>> 5 files changed, 356 insertions(+), 298 deletions(-)
> >>>>
> >>> [...snip...]
> >>>
> >>> Hmmm ... I like that we're trying to clean up the formatting
> further.
> >>> But I'm not too happy that uncrustify seems to disagree slightly ...
> >>> See the attached patch what happened after applying your patch
> and then
> >>> running:
> >>>
> >>> $ uncrustify --no-backup -l C $files
> >>>
> >>> We should either see if our uncrustify config is correct or need
> slight
> >>> adjustments (without needing to re-run it on the complete tree
> once again)
> >>
> >> This seems to be due to 2 things:
> >>
> >> 1) I adhered to the CodeStyle page 'When lines exceed this length,
> >> wrap them using a double indent (ie 8 spaces)', while the crustify
> >> config uses a single indent. Since this double indent was somewhat
> >> arbitrary, I think we should just change the CodeStyle page to single
> >> indent.
> >
> > And this goes against what was agreed upon in the Munich 2014
> hackathon:
> >
> > o line length?
> > - breaking a line in the middle: align to opening (round)
> brackets
> > in line above
> >
> > <http://community.openvpn.net/openvpn/wiki/MunichHackathon2014
> <http://community.openvpn.net/openvpn/wiki/MunichHackathon2014>>
>
> That's about 2), not 1), but I get the point. As before, I'll just have
> to accept the "we decided on this before argument", and deal with it.
> (I'm not sure how to keep the code readable with this requirement, but I
> guess we'll find out.)
>
>
> Do we have to be so strict about the coding style which itself is so
> ill-defined?
> I'll happily adhere to any style if that becomes a requirement for
> patches, but
> overly depending on an external tool to ensure a style that's not even
> precisely
> defined by the CodeStyle page (except by the uncrustify config) sounds lame
> to me.
>
> Aiming towards a uniform coding style is a good goal. Requiring that
> patches should be invariant under "uncrustify -c some-config" is not, imho.
Oh, I'm definitely not in favour of rejecting patches if they don't pass
"uncrustify -c some-config". And I don't think we ever agreed to do so.
But in this case, the 'violations' were all either violations of what we
apparently agreed to in Munich ( 'align with (' ), or simply indented
with a different value than uncrustify did. So actually quite well defined.
-Steffan
------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
