Subject: Re: [bug#68073] [PATCH] Add config-file configuration option to dockerd

From: Mathieu Othacehe <[email-masked]>
Date: Thu, 28 Dec 2023 10:35:40 +0100

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