[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [avrdude-dev] Updated patch for Linux sysfs GPIO programmer type
From: |
Joerg Wunsch |
Subject: |
Re: [avrdude-dev] Updated patch for Linux sysfs GPIO programmer type |
Date: |
Wed, 2 Jan 2013 21:39:27 +0100 |
User-agent: |
Mutt/1.5.20 (2009-06-14) |
Hi Radoslav,
> I have made some changes to the updated version, like renaming the
> programmer type from gpio to linuxgpio, so it's clear it's Linux
> specific.
This is a really good idea!
> Also, I've included the changes to the documentation (man
> page, info page and NEWS file) as you requested some time ago.
That's fine, yes. (Please avoid putting unrelated changes into a
patch though. I've rather fixed the small typo separately.)
> Please, have a look at the attached preliminary version and let me
> know if it fits the requirements to be included in the official
> avrdude release.
>From an eyeball-review, it looks mostly good, thanks!
Two minor remarks:
. Better use 2013 as the copyright year for new files ;), and there's
no need to attribute new files (like the header file) to Brian Dean
rather than yourself (or any actual contributors).
. I'd prefer to reverse the autoconf logic for the enable option, so
it is disabled by default. The feature is only useful for a small
subset of AVRDUDE users anyway, and all those who are candidate
users of it are likely to compile their own version from sources
anyway, so they can manually enable it without problems.
If you don't want this for some reason, add more autoconf hacks to
detect whether the environment is actually linuxgpio-ready. As it
is now, the feature would be enabled by default, thus breaking
compilation for non-Linux systems, and adding unusable bloat into
the resulting binary even for most normal PC-Linux users.
--
cheers, J"org .-.-. --... ...-- -.. . DL8DTL
http://www.sax.de/~joerg/ NIC: JW11-RIPE
Never trust an operating system you don't have sources for. ;-)