All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Lindig <christian.lindig@citrix.com>
To: Edwin Torok <edvin.torok@citrix.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: Ian Jackson <Ian.Jackson@citrix.com>,
	"dave@recoil.org" <dave@recoil.org>,  "wl@xen.org" <wl@xen.org>
Subject: Re: [PATCH v1 0/6] tools/ocaml/xenstored: simplify code
Date: Tue, 18 Aug 2020 09:25:07 +0000	[thread overview]
Message-ID: <1597742707142.74318@citrix.com> (raw)
In-Reply-To: <cbb2742191e9c1303fdfd95feef4d829ecf33a0d.camel@citrix.com>

I see little reason to support old OCaml releases and requiring OCaml 4.06 would be fine with me but I assume that the project might have its own ideas about this.

________________________________________
From: Edwin Torok
Sent: 18 August 2020 08:28
To: Christian Lindig; xen-devel@lists.xenproject.org
Cc: Ian Jackson; dave@recoil.org; wl@xen.org
Subject: Re: [PATCH v1 0/6] tools/ocaml/xenstored: simplify code

On Mon, 2020-08-17 at 14:56 +0200, Christian Lindig wrote:
> This all looks good - I left a small comment on one of the patches
> and I agree that this needs testing. I also wonder about
> compatibility with earlier OCaml releases that we support but I see
> no real obstacles.
>

I've developed the series using OCaml 4.08.1. I think the newest
feature I used was Map.update (OCaml 4.06, nearly 3 years ago).
Looking through https://repology.org/project/ocaml/versions I'm not
sure if we can require more than 4.05 though.
The README in Xen doesn't specify a minimum version, but configure
checks for >=4.02.

I can try to backport my series to OCaml 4.05 (to use Map.find_opt
instead of Map.update) and update the configure check to require 4.05.
It would be possible to backport even further to 4.02 by introducing
additional inefficiencies (Map.mem + Map.find would traverse the map
twice, and Map.find on its own would raise an exception on Not found,
which is more costly than returning None in Map.find_opt), I'd avoid
doing that.

Xen's CI from automation might need some updates to use latest stable
versions:
* Fedora 29 is EOL, should use at least Fedora 31
* Debian Jessie is EOL. Stretch is present, but Buster is missing

Best regards,
--Edwin


  reply	other threads:[~2020-08-18  9:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-14 22:11 [PATCH v1 0/6] tools/ocaml/xenstored: simplify code Edwin Török
2020-08-17 10:12 ` Christian Lindig
2020-08-17 12:56 ` Christian Lindig
2020-08-18  7:28   ` Edwin Torok
2020-08-18  9:25     ` Christian Lindig [this message]
2020-08-18 12:40       ` Andrew Cooper
2020-08-27  9:41         ` Wei Liu
2020-08-27 10:11           ` Edwin Torok

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

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

  git send-email \
    --in-reply-to=1597742707142.74318@citrix.com \
    --to=christian.lindig@citrix.com \
    --cc=Ian.Jackson@citrix.com \
    --cc=dave@recoil.org \
    --cc=edvin.torok@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.