delorie.com/archives/browse.cgi   search  
Mail Archives: geda-user/2020/11/30/15:51:07

X-Authentication-Warning: delorie.com: mail set sender to geda-user-bounces using -f
X-Recipient: geda-user AT delorie DOT com
Date: Mon, 30 Nov 2020 21:31:18 +0100 (CET)
From: Roland Lutz <rlutz AT hedmen DOT org>
To: "Glenn (glimrick AT epilitimus DOT com) [via geda-user AT delorie DOT com]" <geda-user AT delorie DOT com>
Subject: Re: [geda-user] SAB processing patches
In-Reply-To: <e4ff3c96-939b-a93e-a32f-5e938b6daa63@epilitimus.com>
Message-ID: <alpine.DEB.2.21.2011302045040.2894@nimbus>
References: <e4ff3c96-939b-a93e-a32f-5e938b6daa63 AT epilitimus DOT com>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
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

Hi Glenn,

On Mon, 30 Nov 2020, Glenn (glimrick AT epilitimus DOT com) [via 
geda-user AT delorie DOT com] wrote:
> Just posted the patches to add SAB processing to geda/gaf as Bug
> #1906297.

that's an interesting series of patches!  I took the liberty of uploading 
them to Github for easier reference:

     https://github.com/rlutz/geda-gaf/commits/glenn-sab

From a first glance, this looks good.  I'll have to go through the patches 
and understand how they work in detail in order to say anything 
substantiated, but there's some formal things I noticed when skimming 
through:

- Functions which are only used inside the file in which they are defined 
should be "static" (not visible in the global namespace).

- New C code should follow the C coding style of the project:
   - two spaces indentation,
   - one space character before a parameter list(!) and before and after an
     operator, and before (but not after) a pointer asterisk,
   - the opening brace goes on the same line for control structures but on
     a separate line for function headers.
   Code on one level of indentation should always be indented consistently.

- The Python code formatting looks good, but here, too, a space character 
should go before and after operators.

- Your patches contain whitespace errors (trailing whitespace, missing or 
extra newline characters at the end of the file).  You can check your 
commits with ``git diff-tree --check master..''.

- Lines in source and text files should be less than 80 characters long. 
Lines in commit messages should not be longer than 72 characters, and (if 
one feels pedantic) 64 characters on the first line.

- You don't have to list the files you touched in the commit message.

Don't let yourself discourage by this!  These are just conventions.  I'll 
give you more detailed feedback as soon as I find time.

Roland

- Raw text -


  webmaster     delorie software   privacy  
  Copyright © 2019   by DJ Delorie     Updated Jul 2019