From owner-freebsd-ports@FreeBSD.ORG Thu Jan 24 14:16:33 2013 Return-Path: Delivered-To: freebsd-ports@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id AF8F4849 for ; Thu, 24 Jan 2013 14:16:33 +0000 (UTC) (envelope-from freebsd@grem.de) Received: from mail.grem.de (outcast.grem.de [213.239.217.27]) by mx1.freebsd.org (Postfix) with SMTP id F23EBA03 for ; Thu, 24 Jan 2013 14:16:32 +0000 (UTC) Received: (qmail 19482 invoked by uid 89); 24 Jan 2013 14:16:30 -0000 Received: from unknown (HELO bsd64.grem.de) (mg@grem.de@194.97.158.66) by mail.grem.de with ESMTPA; 24 Jan 2013 14:16:30 -0000 Date: Thu, 24 Jan 2013 15:16:29 +0100 From: Michael Gmelin To: Dag-Erling =?UTF-8?B?U23DuHJncmF2?= Subject: Re: Using bidirectional authentication in pkgng Message-ID: <20130124151629.588c33c8@bsd64.grem.de> In-Reply-To: <86r4lau2wh.fsf@ds4.des.no> References: <20130118035721.283135fb@bsd64.grem.de> <50F9B6CC.3040303@infracaninophile.co.uk> <20130122193035.4c51be04@bsd64.grem.de> <20130123004147.GG27275@ithaqua.etoilebsd.net> <86d2wuvrjg.fsf@ds4.des.no> <20130124121942.07436be3@bsd64.grem.de> <86r4lau2wh.fsf@ds4.des.no> X-Mailer: Claws Mail 3.9.0 (GTK+ 2.24.6; amd64-portbld-freebsd9.0) Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: freebsd-ports@freebsd.org X-BeenThere: freebsd-ports@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Porting software to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Jan 2013 14:16:33 -0000 On Thu, 24 Jan 2013 14:27:42 +0100 Dag-Erling Sm=C3=B8rgrav wrote: > Michael Gmelin 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. >=20 > 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? >=20 > Just a quick summary of issues with your patch: >=20 > - There are many instances of misplaced opening / closing braces. >=20 > - 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. >=20 > - 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. >=20 > - 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. >=20 > 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. >=20 > DES Cheers, --=20 Michael Gmelin