Using bidirectional authentication in pkgng

[Michael Gmelin]

On Thu, 24 Jan 2013 14:27:42 +0100
Dag-Erling Smørgrav <des at des.no> wrote:

> Michael Gmelin <freebsd at grem.de> writes:
> > Would be great if you could point out the exact issues, so I could
> > avoid them next time (I spent literally hours trying to clean up
> > the code so it complies to style(9), even though it doesn't seem
> > like fetch really follows it either). Other people's coding
> > standards are always arbitrary and, um, wrong anyway, you know ;)

Some remarks below, not supposed to sound whiny, just want to
understand where certain things are specified and if there might be
some need to improve documentation to make it easier for contributors.
It's not that amusing to spend a substantial amount of time on
formatting your code in a coding standard you're not familiar with just
to hear that it's completely wrong. Ultimately I'm aiming to provide
patches that conform to the project's style expectations.

> 
> libfetch follows style(9) very closely, except in some parts of http.c
> which are too deeply nested.

http.c makes heavy use of initialization in declarations (which I'm a
big fan of), but which is discouraged by style(9) ("Be careful to not
obfuscate the code by initializing variables in the declarations."). I
spent some time removing those from my code (even though I thought it
looks better with them), so maybe it's not as strict as I thought it
is - ultimately "obfuscate" is kind of a subjective term?

> 
> Just a quick summary of issues with your patch:
> 
>  - There are many instances of misplaced opening / closing braces.
> 
>  - Several lines are too long, and almost all your continuation lines
>    are misindented.

Hm, I used an emacs mode that was supposed to indent according to
style(9) - so maybe that wasn't the right tool. Do you have a link to
an emacs mode that works like expected? (I clearly won't do these
indentations manually, they're so counter-intuitive to me that I
would mess them up badly - it's hard to work against your organization's
usual coding style and switching back and forth). Any recommended
combination of lint/indent/whatever I could use to validate the code
automatically?

I just checked, all lines I added are <80 characters. Style(9) itself
doesn't specify any line length limitations, so I figured staying below
80 is already quite conservative (or are you going really old school
and limit yourself to 72?). Since I found quite a number of lines
that are longer than that I figured it would be ok.

> 
>  - There are many instances of missing whitespace around operators in
>    expressions.

Thought I caught them all ;) I clearly forgot about "for" loops -
force of habit, sorry.

> 
>  - You declare variables inside blocks, and declarations in general
> are not properly sorted.

I wasn't aware that this is not allowed (it's such an incredibly
useful feature to me). Could you point me to the section in style(9)
that specifies this (couldn't find it)? In regards of declaration sort
order in style(9) I read up on it and understand now what's expected.

> 
> There are issues with the man page as well - you didn't bump the date,
> sentences don't begin on new lines, and some lines are too long - but
> you get a million bonus points for updating it at all.

Do you have a good pointer to style guides for man pages/nroff? Didn't
bump the date since I figured it won't be accepted unchanged anyway.

> 
> DES

Cheers,

-- 
Michael Gmelin