unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Julien Lepiller <julien@lepiller.eu>
To: aecepoglu <aecepoglu@fastmail.fm>
Cc: 45762@debbugs.gnu.org
Subject: [bug#45762] [PATCH] ocaml: Add ocaml-down and ocaml-ocp-index
Date: Mon, 11 Jan 2021 16:15:14 +0100	[thread overview]
Message-ID: <20210111161514.18648234@tachikoma.lepiller.eu> (raw)
In-Reply-To: <20210110104915.2t6u4atuikypepfw@minitower>

Hi,

thanks for the patch!

Since it adds three packages, could you make this a series of three
patches, where each adds just one package? First, ocp-indent, then
ocp-index and finally down. Below are remarks that you should address in
these patches too :)

For each of them, the title of the patch should be "gnu: Add
<package-name>.", followed by a blank line and "*
gnu/packages/ocaml.scm (<package-name>): New variable.".

Le Sun, 10 Jan 2021 13:49:15 +0300,
aecepoglu <aecepoglu@fastmail.fm> a écrit :

> ---
>  gnu/packages/ocaml.scm | 132
> +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 132
> insertions(+)
> 
> diff --git a/gnu/packages/ocaml.scm b/gnu/packages/ocaml.scm
> index 2df16e7cd2..6699eb1050 100644
> --- a/gnu/packages/ocaml.scm
> +++ b/gnu/packages/ocaml.scm
> @@ -390,6 +390,71 @@ repository-wide uninstallability checks.")
>      ;; with static-linking exception
>      (license license:lgpl2.1+)))

In the first patch of the series, could you add a copyright line for
yourself? It's fine to use a pseudonym if you don't want to use your
legal name.

>  
> +(define-public ocaml-down
> +  (package
> +    (name "ocaml-down")
> +    (version "0.0.3")
> +    (source
> +      (origin
> +        (method url-fetch)
> +        (uri
> "https://erratique.ch/software/down/releases/down-0.0.3.tbz")
> +        (sha256
> +          (base32
> +
> "1nz2f5j17frgr2vrslcz9klmi6w9sm2vqwwwpi33ngcm3rgmsrlg"))))
> +    (build-system ocaml-build-system)
> +    (arguments
> +     `(#:tests? #f

Please add a comment explaining why you skip tests (could be "no tests"
if there is no tests) :)

> +       #:phases (modify-phases %standard-phases
> +                              (delete 'configure))
> +       #:build-flags (list "build"
> +                           "--lib-dir" (string-append (assoc-ref
> %outputs "out")
> +
> "/lib/ocaml/site-lib"))))

I think the indentation is off here, but I'll fix it, don't worry.

> +    (native-inputs
> +      `(("ocaml-findlib" ,ocaml-findlib)
> +        ("ocamlbuild" ,ocamlbuild)
> +        ("ocaml-topkg" ,ocaml-topkg)
> +        ("opam" ,opam)))

Same here, there's one space too much below native-inputs.

> +    (home-page "https://erratique.ch/software/down")
> +    (synopsis "An OCaml toplevel (REPL) upgrade")

Synpsis and description should not start with an article, so make
it "OCaml toplevel (REPL) upgrade" :)

> +    (description
> +      "
> +Down is an unintrusive user experience upgrade for the `ocaml`
> +toplevel (REPL). 
> +
> +Simply load the zero dependency `Down` library in the `ocaml`
> toplevel +and you get line edition, history, session support and
> identifier +completion and documentation (courtesy of
> [`ocp-index`][ocp-index]). +
> +Add this to your `~/.ocamlinit`:
> +
> +    #use \"down.top\"
> +
> +You may also need to add this to your `~/.ocamlinit` and declare
> +the environment variable OCAML_TOPLEVEL_PATH
> +
> +    let () =
> +      try Topdirs.dir_directory (Sys.getenv \"OCAML_TOPLEVEL_PATH\")
> +      with Not_found -> ()
> +
> +OR
> +     
> +    let () = String.split_on_char ':' (Sys.getenv \"OCAMLPATH\")
> +             |> List.filter (fun x -> Filename.check_suffix x
> \"/site-lib\")
> +             |> List.map (fun x -> x ^ \"/toplevel\")
> +             (* remove the line below if you don't want to see the
> text
> +                every time you start the toplevel *)
> +             |> List.map (fun x -> Printf.printf \"adding directory
> %s\\n\" x; x)
> +             |> List.iter Topdirs.dir_directory;;
> +
> +
> +![tty](doc/tty.png)
> +
> +Down is distributed under the ISC license.
> +
> +
> +[ocp-index]: https://github.com/OCamlPro/ocp-index")
> +    (license license:isc)))
> +

I supposed you used the description from the opam importer? It doesn't
use the same syntax as guix, so you have to translate them usually.
Don't worry too much though if you can't make it, I'll take care of it
when applying your patch :)

Basically, don't start with a new line, links are not supported, and
generally use texinfo syntax.

>  (define-public ocaml-opam-file-format
>    (package
>      (name "ocaml-opam-file-format")
> @@ -2363,6 +2428,73 @@ the JSON data format.  It can process JSON
> text without blocking on IO and without a complete in-memory
> representation of the data.") (license license:isc)))
>  
> +(define-public ocaml-ocp-indent
> +  (package
> +    (name "ocaml-ocp-indent")
> +    (version "1.8.1")
> +    (source
> +      (origin
> +        (method url-fetch)
> +        (uri
> "https://github.com/OCamlPro/ocp-indent/archive/1.8.1.tar.gz")
> +        (sha256
> +          (base32
> +
> "0rqcvwzpwwzwqvrzg1l37jjiglrk2ijgjg1c7vy3xvc8lk9zyysw"))))
> +    (build-system dune-build-system)
> +    (arguments
> +     `(#:tests? #f

Same question, why disable tests?

> +       #:build-flags (list "--profile=release")))

Unless there's a specific reason, I think #:package the usual way to do
it.

> +    (propagated-inputs
> +      `(("ocaml-cmdliner" ,ocaml-cmdliner)))
> +    (home-page
> +      "http://www.typerex.org/ocp-indent.html")
> +    (synopsis
> +      "A simple tool to indent OCaml programs")

Remove the article, so "Simple tool to indent OCaml programs"

> +    (description
> +      "Ocp-indent is based on an approximate, tolerant OCaml parser
> and a simple stack +machine ; this is much faster and more reliable
> than using regexps. Presets and +configuration options available,
> with the possibility to set them project-wide. +Supports most common
> syntax extensions, and extensible for others. +
> +Includes:
> +- An indentor program, callable from the command-line or from within
> editors +- Bindings for popular editors
> +- A library that can be directly used by editor writers, or just
> for\n  fault-tolerant/approximate parsing. +")

Sorry, my client is making a mess out of this. I'll make sure it looks
alright, don't worry about it.

> +    (license #f)))

Can you check the license and fill in the field please? #f is not
acceptable in Guix :)

> +
> +(define-public ocaml-ocp-index
> +  (package
> +    (name "ocaml-ocp-index")
> +    (version "1.2.1")
> +    (source
> +      (origin
> +        (method url-fetch)
> +        (uri
> "https://github.com/OCamlPro/ocp-index/archive/1.2.1.tar.gz")
> +        (sha256
> +          (base32
> +
> "169msb3k1ip5lp79lr62nsk715rwrn53x9jcrnxz4js7c0fv0cyr"))))
> +    (build-system dune-build-system)
> +    (arguments
> +     `(#:package "ocp-index"))
> +    (propagated-inputs
> +      `(("ocaml-ocp-indent" ,ocaml-ocp-indent)
> +        ("ocaml-re" ,ocaml-re)
> +        ("ocaml-cmdliner" ,ocaml-cmdliner)))
> +    (native-inputs `(("ocaml-cppo" ,ocaml-cppo)))
> +    (home-page
> +      "http://www.typerex.org/ocp-index.html")
> +    (synopsis
> +      "Lightweight completion and documentation browsing for OCaml
> libraries")
> +    (description
> +      "This package includes
> +* The `ocp-index` library and command-line tool
> +* `ocp-grep`, a tool that finds uses of a given (qualified)
> identifier in a source tree +* bindings for emacs and vim (sublime
> text also
> [available](https://github.com/whitequark/sublime-ocp-index/)) + +To
> automatically configure your editors, install this with package
> `user-setup`. +")

My client is again making a mess out of this, but I'll fix this anyway,
don't worry.

> +    (license #f)))

Same, please specify the license.

> +
>  (define-public ocaml-ocurl
>    (package
>      (name "ocaml-ocurl")

Overall great contribution! Thank you again!




  parent reply	other threads:[~2021-01-11 15:16 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-10 10:49 [bug#45762] [PATCH] ocaml: Add ocaml-down and ocaml-ocp-index aecepoglu
2021-01-11 14:57 ` zimoun
2021-01-11 15:15 ` Julien Lepiller [this message]
2021-01-11 20:55 ` [bug#45762] [PATCH 1/3] ocaml: Add ocaml-ocp-indent * gnu/packages/ocaml.scm (ocaml-ocp-indent): New variable aecepoglu
2021-01-11 20:55 ` [bug#45762] [PATCH 2/3] ocaml: Add ocaml-ocp-index * gnu/packages/ocaml.scm (ocaml-ocp-index): " aecepoglu
2021-01-11 20:56 ` [bug#45762] [PATCH 3/3] ocaml: Add ocaml-down * gnu/packages/ocaml.scm (ocaml-down): " aecepoglu
2021-01-12 14:10 ` bug#45762: [PATCH] ocaml: Add ocaml-down and ocaml-ocp-index Julien Lepiller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210111161514.18648234@tachikoma.lepiller.eu \
    --to=julien@lepiller.eu \
    --cc=45762@debbugs.gnu.org \
    --cc=aecepoglu@fastmail.fm \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).