[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: imagemagick branch
From: |
Stefan Monnier |
Subject: |
Re: imagemagick branch |
Date: |
Tue, 18 May 2010 12:35:34 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) |
> 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