On Thu, Dec 15, 2016 at 8:00 AM, David Sommerseth <
open...@sf.lists.topphemmelig.net> wrote:

> > The only issue I noticed is that some functions, for loops and switch
> > statements have their opening braces on the same line
> > ssl.c line 272, tun.c line 697 and many such in the several files.
> > (try grep ") {$"  *.c).
> >
> > Probably needed nl_switch_brace, nl_fcall_brace etc in the config --
> > urgh.. Too many options is a blessing and bane at the same time..
>
> Agreed.  I think that we can do some minor reformatting changes later on
> if we decide to change some more options.  Those reformatting changes
> will most likely not be as invasive as this one has been.
>

There are also some multi-line comments with closing "*/" not on a line by
itself.

If we want to fix any of this I think this is the time -- massive
reformatting is not something to be done often.


> > This is not a show stopper for me. Could be handled manually in future
> > edits when code in the vicinity is touched..
>
> I will now write a new script which patch contributors should run
> against modified files before committing and submitting.  This should
> hopefully make it better in the long run.  But with that in mind, we
> will need to re-run the reformat-all.sh script each time we decide to
> modify uncrustify options.


We have to be cautious here. Running uncrustify on is not something we
should do frequently -- I thought this was a once in a lifetime
reformatting to be followed by some self-imposed discipline for future
patches guided by the CodeStyle page.

First, uncrustify is not idempotent, although running two or three times
appears to make the source settle into an invariant form. Sometimes
multiple runs slightly messes up some otherwise good formatting.  Secondly,
its an actively developed code so behaviour could change. In fact they
already fixed the segfault bug that I had reported upstream.

So I'd be wary of adopting a policy that patch submission be preceded by an
uncrustify run.

Selva
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to