Re: imagemagick branch

[joakim@verona.se]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> 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.

I refreshed the branch and tried to address the issues you raised.
Could you have a look again to see if its merge-worthy?

>
>> 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
-- 
Joakim Verona