On 03.09.2018 23:36, Ludovic Courtès wrote:
> Hello Tim,
> 
> Tim Gesthuizen <tim.gesthuizen@yahoo.de> skribis:
> 
>> The attached patch adds emacs-irony-mode.
>> It is also packaged in MELPA so it is definitely free software.
>> If there are problems with the description or synopsis just let me know
>> and I will change the patch accordingly.
> 
> Thanks for the patch!  I have some suggestions and comments below, but
> overall it LGTM:
> 
>> From 6975ba9e4b005c77f00d7f2187b5d8047f15ba07 Mon Sep 17 00:00:00 2001
>> From: Tim Gesthuizen <tim.gesthuizen@yahoo.de>
>> Date: Thu, 30 Aug 2018 17:39:57 +0200
>> Subject: [PATCH] gnu: Add emacs-irony-mode
>>
>> ---
>>  gnu/packages/emacs.scm | 49 ++++++++++++++++++++++++++++++++++++------
> 
> Please run ‘git log gnu/packages/emacs.scm’ to see our conventions for
> commit logs, or see
> <https://www.gnu.org/prep/standards/html_node/Change-Logs.html>.  (We can
> always fix it up for you though, it’s no big deal.)
> 
>> +    (home-page "https://github.com/Sarcasm/irony-mode")
>> +    (synopsis "Clang autocompletion and syntax checking integration for GNU Emacs")
> 
> It’s a bit long.  Perhaps: “Code completion and syntax checks for Emacs”?
> 
>> +    (description "Provides clang assisted syntax checking and autocompletion
>> + for C,C++ and ObjC.")
> 
> Please make a full sentence, as per
> <https://www.gnu.org/software/guix/manual/en/html_node/Synopses-and-Descriptions.html>.
> 
>> +    (license license:gpl3)))
> 
> Isn’t it ‘gpl3+’, meaning “version 3 or any later version, at your
> option”?
> 
>> +(define-public emacs-irony-mode-server
>> +  (package (inherit emacs-irony-mode)
>> +    (name "emacs-irony-mode-server")
>> +    (propagated-inputs
>> +     `(("clang" ,clang)))
> 
> Instead of propagating Clang, which clutters the user’s profile, do you
> think we could patch the .el files such that they refer to ‘clang’ by
> its absolute file name?  See ‘emacs-emms’ for an example of that.
> 
>> +    (arguments
>> +     `(#:phases
>> +       (modify-phases %standard-phases
>> +         (replace 'configure
>> +           (lambda* (#:key outputs #:allow-other-keys)
>> +             (let ((out (assoc-ref outputs "out")))
>> +               (invoke "cmake"
>> +                       "server"
>> +                       (string-append "-DCMAKE_INSTALL_PREFIX=" out))))))))
> 
> Please return #t at the end of the phase.
> 
>>      (source (origin
>> -	      (method url-fetch)
>> -	      (uri (string-append "https://github.com/smihica/emmet-mode"
>> -				  "/archive/" version ".tar.gz"))
>> +              (method url-fetch)
>> +              (uri (string-append "https://github.com/smihica/emmet-mode"
>> +                                  "/archive/" version ".tar.gz"))
>>                (file-name (string-append name "-" version ".tar.gz"))
>> -	      (sha256
>> -	       (base32
>> -		"0g3p22yabfcp98cfv9dgl9il2m2pd53isq2q11vb3s7qyn31f7zj"))))
>> +              (sha256
>> +               (base32
>> +                "0g3p22yabfcp98cfv9dgl9il2m2pd53isq2q11vb3s7qyn31f7zj"))))
> 
> This change is unnecessary and unrelated; could you remove it?
> 
> Could you send an updated patch?
> 
> Thank you!
> 
> Ludo’.
> 

Thank you for your suggestions.
Changed patch accordingly.
clang is used via libclang from emacs-irony-mode-server which is linked
dynamically. Because of this I could just set the propagated-input to a
normal input. It is linked automatically to the version in /gnu/store.
Because of some weird  problems I had today with building guix from git
I could not verify the patch.
Please check that it is working before pushing it.