all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Mathieu Othacehe <othacehe@gnu.org>
To: Connor Clark <connor@psyleft.com>
Cc: zimon.toutoune@gmail.com, paren@disroot.org, ludo@gnu.org,
	me@tobias.gr, rekado@elephly.net, rg@raghavgururajan.name,
	jgart@dismail.de, guix@cbaines.net, 68073@debbugs.gnu.org
Subject: [bug#68073] [PATCH] Add config-file configuration option to dockerd
Date: Thu, 28 Dec 2023 10:35:40 +0100	[thread overview]
Message-ID: <87plyq7jpv.fsf@gnu.org> (raw)
In-Reply-To: <20231227203015.14265-1-connor@psyleft.com> (Connor Clark's message of "Wed, 27 Dec 2023 15:20:21 -0500")


Hello Connor,

Thanks for this first submission and welcome :)

> This is my first time submitting a patch here, so please let me know
> if I'm missing something important, or my email is formatted
> incorrectly, or anything else.

This looks OK, a few remarks below:

> This patch adds an option to pass a json configuration file directly into
> dockerd for options which aren't available in the docker-configuration record,
> of which there are many. From what I've seen, some other packages use a
> raw-configuration-string that gets appended into a file to accomplish similar
> things, but I figured a file-like was better here because the rest of the
> options are passed into the command invocation and not serialized into a file.

You are missing a commit message. Its format is described here:
https://www.gnu.org/prep/standards/html_node/Change-Logs.html#Change-Logs

Here that would look like:

--8<---------------cut here---------------start------------->8---
* gnu/services/docker.scm (docker-configuration)[config-file]: New
file-like field.
...
--8<---------------cut here---------------end--------------->8---

You can have a look to the git log of that specific file for examples.

If you want extra points, you can extend the system test in
gnu/tests/docker.scm to check that the docker-service-type is honoring
this new config-file field. Here is the documentation of system tests:
https://guix.gnu.org/manual/en/html_node/Running-the-Test-Suite.html

Could you please send a v2?

Thanks,

Mathieu




  reply	other threads:[~2023-12-28  9:36 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-27 20:20 [bug#68073] [PATCH] Add config-file configuration option to dockerd Connor Clark via Guix-patches via
2023-12-28  9:35 ` Mathieu Othacehe [this message]
2023-12-28  9:39   ` Mathieu Othacehe
2023-12-29  4:47 ` [bug#68073] [PATCH v2] services: docker: Add config-file option Connor Clark via Guix-patches via
2024-01-03 14:31   ` bug#68073: " Mathieu Othacehe

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=87plyq7jpv.fsf@gnu.org \
    --to=othacehe@gnu.org \
    --cc=68073@debbugs.gnu.org \
    --cc=connor@psyleft.com \
    --cc=guix@cbaines.net \
    --cc=jgart@dismail.de \
    --cc=ludo@gnu.org \
    --cc=me@tobias.gr \
    --cc=paren@disroot.org \
    --cc=rekado@elephly.net \
    --cc=rg@raghavgururajan.name \
    --cc=zimon.toutoune@gmail.com \
    /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 external index

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

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.