X-Authentication-Warning: delorie.com: mail set sender to geda-user-bounces using -f
X-Recipient: geda-user AT delorie DOT com
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=google.com; s=20120113;
        h=mime-version:x-originating-ip:in-reply-to:references:from:date
         :message-id:subject:to:content-type:x-gm-message-state;
        bh=sHMdot0P34xqBbMkoocQ+gSAkwdBJEdWkTlEBkfmECk=;
        b=R+ISGmYPhv2FIsIAvX0PqDyonB26qvWtNoQfKI61iYB34eU7kFt6Xm/SkDdV/ezq8J
         Nh9gTE2dLSWgaMJSRKq1oPLS7f7HKleIyBR8groR+BB3yBJ8QUpJLlTeV6/+8BgDfAes
         1/s9SxJVIKpSQD3ltz/uxoW13ScjZj4zxkqdUShetc36yjfgql8RSpDSpmzQQR4/IwvG
         lWaX6zrWxXsu+B5fTuM4pISH3y0Y4MKvCe+7hAv5nMzPfJweZ54dq+IK2Spvl7vbpepC
         /8NFqOlM7pfODLNCpvZDRJiAcY3lSTTQY+bZ+tmBAtqbqBneeDTrDO7teWBW+yT12umC
         gljQ==
MIME-Version: 1.0
X-Originating-IP: [206.220.194.212]
In-Reply-To: <20120614194141.GA22067@malakian.lan>
References: <CA+DWcQZO+-=umy4LdB6MXnKc=uGpLp1M7k392cnigVmzGDWy2g AT mail DOT gmail DOT com>
 <20120529092557 DOT 62e0178d AT svelte> <CA+DWcQYSXxvKbcHgFknZ_tyGiTMJj8OvTE4H0csF7gYRD0saFA AT mail DOT gmail DOT com>
 <20120530140222 DOT GA21786 AT visitor2 DOT iram DOT es> <CA+DWcQZ3Wt_xK_3P6OsHxzSaSrO+qyrqCd7YE-e75aLT1ZaPyg AT mail DOT gmail DOT com>
 <CA+DWcQa0bqv5yxzEjQ+09=bPku--u7HnZgXmKx4AUYfmWTCrVQ AT mail DOT gmail DOT com>
 <20120608215309 DOT GC7353 AT malakian DOT lan> <CA+DWcQasatN9TiG0XToxmDZTonCgz0=vETxDKABmzMc-tHqH=A AT mail DOT gmail DOT com>
 <CA+DWcQby6+hNQKJaUqhGBgVrDZsYKodypC6+8Z-AV+0HU7e3aw AT mail DOT gmail DOT com>
 <20120609184751 DOT GA9728 AT malakian DOT lan> <20120614194141 DOT GA22067 AT malakian DOT lan>
From: Benjamin Bergman <ben AT benbergman DOT ca>
Date: Thu, 14 Jun 2012 17:44:37 -0500
Message-ID: <CA+DWcQZNtW-auOB078wnj+L0O3Mkyp=t--SQcOxhn3_dtC=-Bg@mail.gmail.com>
Subject: Re: [geda-user] extra mask, plating, and silk colours in pcb
To: geda-user AT delorie DOT com
Content-Type: multipart/alternative; boundary=f46d04088c759e34c304c2767315
X-Gm-Message-State: ALoCoQlfYkG6CQ6GKX0uu1VNPiZQeF3z8/mQ42sapkU49xSzEOJ7Fc/95a8EgZGjLqMMq3Fb0qcy
Reply-To: geda-user AT delorie DOT com
Errors-To: nobody AT delorie DOT com
X-Mailing-List: geda-user AT delorie DOT com
X-Unsubscribes-To: listserv AT delorie DOT com
Precedence: bulk

--f46d04088c759e34c304c2767315
Content-Type: text/plain; charset=ISO-8859-1

On Thu, Jun 14, 2012 at 2:41 PM, Andrew Poelstra <asp11 AT sfu DOT ca> wrote:

> I have made the following changes to your patch:
>
>
>  1. Consolidated all 17 patches into one. They all fundamentally do
>     a single thing (add a color-changing feature), all affect the same
>     source file, and the total diff is only ~300 lines. I think this is
>     reasonable.
>
>     The 17-patch version was over 1000 lines, and largely redundant.
>     (For example, one patch introduced a compilation warning, which
>      the next patch got rid of.)
>

Makes sense. No one needs to see my intermediate, buggy commits.

 2. Fixed whitespace. There should be no tabs in source files -- the
>     ones that are there were put in by errant tools, and represent
>     8 spaces. So if you're editing code with tabs in it, check that
>     your editor has its tabstops set to 8 spaces, so that the spacing
>     will look right on your machine.
>
>     (Or even better, submit a separate whitespace-fixing patch to get
>      rid of the tabs.)
>

Good to know. I use vim and have it set to convert tabs to 2 space
characters. A vim modeline at the end of the file would help other vim
users to maintain correct indentation. Perhaps useful for another future
patch.


>  3. Changed the clip() function to check for both values > 255 and
>     values < 0. Removed the explicit check for < 0 from subtract().
>     This reduces the potential for bugs.
>

Makes sense to me.

The new patch is as follows. If you're happy to have your name on
> it, let me know and I'll push it.
>

I didn't read it word for word, but based on your summaries, I'd be happy
to see it in the official pcb repository :)   Thanks for reviewing it.

Ben

--f46d04088c759e34c304c2767315
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

<div class=3D"gmail_quote">On Thu, Jun 14, 2012 at 2:41 PM, Andrew Poelstra=
 <span dir=3D"ltr">&lt;<a href=3D"mailto:asp11 AT sfu DOT ca" target=3D"_blank">as=
p11 AT sfu DOT ca</a>&gt;</span> wrote:<br><blockquote class=3D"gmail_quote" style=
=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

I have made the following changes to your patch:<br>
<br>
<br>
 =A01. Consolidated all 17 patches into one. They all fundamentally do<br>
 =A0 =A0 a single thing (add a color-changing feature), all affect the same=
<br>
 =A0 =A0 source file, and the total diff is only ~300 lines. I think this i=
s<br>
 =A0 =A0 reasonable.<br>
<br>
 =A0 =A0 The 17-patch version was over 1000 lines, and largely redundant.<b=
r>
 =A0 =A0 (For example, one patch introduced a compilation warning, which<br=
>
 =A0 =A0 =A0the next patch got rid of.)<br></blockquote><div><br></div><div=
>Makes sense. No one needs to see my intermediate, buggy commits.=A0</div><=
div><br></div><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;=
border-left:1px #ccc solid;padding-left:1ex">


 =A02. Fixed whitespace. There should be no tabs in source files -- the<br>
 =A0 =A0 ones that are there were put in by errant tools, and represent<br>
 =A0 =A0 8 spaces. So if you&#39;re editing code with tabs in it, check tha=
t<br>
 =A0 =A0 your editor has its tabstops set to 8 spaces, so that the spacing<=
br>
 =A0 =A0 will look right on your machine.<br>
<br>
 =A0 =A0 (Or even better, submit a separate whitespace-fixing patch to get<=
br>
 =A0 =A0 =A0rid of the tabs.)<br></blockquote><div><br></div><div>Good to k=
now. I use vim and have it set to convert tabs to 2 space characters. A vim=
 modeline at the end of the file would help other vim users to maintain cor=
rect indentation. Perhaps useful for another future patch.</div>

<div>=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;=
border-left:1px #ccc solid;padding-left:1ex">
 =A03. Changed the clip() function to check for both values &gt; 255 and<br=
>
 =A0 =A0 values &lt; 0. Removed the explicit check for &lt; 0 from subtract=
().<br>
 =A0 =A0 This reduces the potential for bugs.<br></blockquote><div><br></di=
v><div>Makes sense to me.=A0</div><div><br></div><blockquote class=3D"gmail=
_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:=
1ex">


The new patch is as follows. If you&#39;re happy to have your name on<br>
it, let me know and I&#39;ll push it.<br></blockquote><div><br></div><div>I=
 didn&#39;t read it word for word, but based on your summaries, I&#39;d be =
happy to see it in the official pcb repository :) =A0 Thanks for reviewing =
it.</div>

<div><br></div><div>Ben</div></div>

--f46d04088c759e34c304c2767315--