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 To: "Glenn (glimrick AT epilitimus DOT com) [via geda-user AT delorie DOT com]" Subject: Re: [geda-user] SAB processing patches In-Reply-To: Message-ID: References: User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed 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 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