Re: [PATCH v3 1/5] config: split out config_parse_options

From: Taylor Blau <me@ttaylorr.com>
Date: Oct 23, 2023, 18:46 UTC
Message-ID: <ZTa/fvfuPN+JJx89@nand.local>
In-Reply-To: <20231023175217.984090-1-jonathantanmy@google.com>

On Mon, Oct 23, 2023 at 10:52:17AM -0700, Jonathan Tan wrote:

Show 10 quoted lines

> Josh Steadmon <steadmon@google.com> writes:
> > From: Glen Choo <chooglen@google.com>
> >
> > "struct config_options" is a disjoint set of options used by the config
> > parser (e.g. event listeners) and options used by config_with_options()
> > (e.g. to handle includes, choose which config files to parse).
>
> Can this sentence be reworded? In particular, "disjoint" is a word
> normally applied to two or more sets (meaning that they have no elements
> in common), but here it is used for only one.

The pedant in me agrees with you. I do think that the sentence reads a little awkwardly. Perhaps instead:

    "struct config_options" has members which serve two distinct
    purposes. There are a set of members used by the configuration parse
    (e.g. event listeners). There is also a set used by
    config_with_options() (e.g to handle includes, choose which config
    files to parse).

> Everything else looks good, and the reasoning (some functions only use
> a subset of the fields, and this subset is easily explained conceptually
> as those related to parsing) makes sense.

Yup.

Thanks, Taylor