From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: imagemagick branch Date: Tue, 18 May 2010 12:35:34 -0400 Message-ID: References: <831vd9bmlg.fsf@gnu.org> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: dough.gmane.org 1274200556 12918 80.91.229.12 (18 May 2010 16:35:56 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Tue, 18 May 2010 16:35:56 +0000 (UTC) Cc: Eli Zaretskii , emacs-devel@gnu.org To: joakim@verona.se Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue May 18 18:35:54 2010 connect(): No such file or directory Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1OEPlq-0003B8-3M for ged-emacs-devel@m.gmane.org; Tue, 18 May 2010 18:35:54 +0200 Original-Received: from localhost ([127.0.0.1]:58561 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OEPlp-00081Q-KJ for ged-emacs-devel@m.gmane.org; Tue, 18 May 2010 12:35:53 -0400 Original-Received: from [140.186.70.92] (port=36109 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OEPli-000818-BZ for emacs-devel@gnu.org; Tue, 18 May 2010 12:35:48 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OEPld-0007ae-G8 for emacs-devel@gnu.org; Tue, 18 May 2010 12:35:46 -0400 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.181]:55855 helo=ironport2-out.pppoe.ca) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OEPlY-0007ZN-7I; Tue, 18 May 2010 12:35:36 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AqQFAJde8ktMCpz0/2dsb2JhbACReYwHcr8khRAEg1+IUA X-IronPort-AV: E=Sophos;i="4.53,256,1272859200"; d="scan'208";a="64482721" Original-Received: from 76-10-156-244.dsl.teksavvy.com (HELO pastel.home) ([76.10.156.244]) by ironport2-out.pppoe.ca with ESMTP; 18 May 2010 12:35:34 -0400 Original-Received: by pastel.home (Postfix, from userid 20848) id 3BC638064; Tue, 18 May 2010 12:35:34 -0400 (EDT) In-Reply-To: (joakim@verona.se's message of "Tue, 18 May 2010 12:41:38 +0200") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:124899 Archived-At: > Yes, the code could probably safely be added to trunk. Even if one > activates imagemagick with "configure --with-imagemagick=yes" > imagemagick wont kick in unless you execute (imagemagick-register-types) I just took a quick look at the code and I see the following nits to fix: - obviously a merge will have to come with a good ChangeLog. - also the merge will need to come with documentation. Maybe not in the Texinfo form yet, but at least in the etc/NEWS with enough info that describes the `scale' and other such arguments that someone can start using them. - the README talks about naming inconsistencies, I think these should be fixed before a first commit (should be straightforward). - the "let" in image.el should not be followed by a line break and the while should be replaced by a dolist. - the prototype of imagemagick_load_image has some odd indentation in its args, not sure what happened. - a few lines in the C code break the 80columns limit. - please use ANSI style function declarations rather than K&R for new code. - you can get rid of the prototypes by reordering the code. - the docstrings in DEFUN should not be indented (they'll display weirdly otherwise in C-h f). - Some "{" are at the end of a for/if rather than on their own line. - why use "*( imtypes + i)" rather than "imtypes[i]"? - some "," lack a space after them. - several "=" and "==" lack spaces around them. > Another question then, how do I merge best to trunk? I'm reading: > http://www.emacswiki.org/emacs/BzrForEmacsDevs#MergeToUpstream > but it suggests removing my local branch after merging. The branch's history is not really interesting it seems, so you might as well do the first commit on trunk as a separate commit rather as a merge. But if you prefer a merge, see Eli's answer. > I would like to continue to add some more experimental functions to > the branch, and later merge these to trunk. Would that also be ok? > Or would it be better to create a new branch for this purpose? No need for a new branch. Stefan