RE: [PATCH v1 1/2] dts: fabric requirements

[Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, April 12, 2023 10:25 AM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: Wathsala Wathawana Vithanage <wathsala.vithanage@arm.com>;
> jspewock@iol.unh.edu; probb@iol.unh.edu; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; lijuan.tu@intel.com;
> bruce.richardson@intel.com; dev@dpdk.org
> Subject: Re: [PATCH v1 1/2] dts: fabric requirements
> 
> 12/04/2023 15:42, Juraj Linkeš:
> > On Tue, Apr 11, 2023 at 4:48 PM Thomas Monjalon <thomas@monjalon.net>
> wrote:
> > >
> > > 04/04/2023 13:51, Juraj Linkeš:
> > > > On Mon, Apr 3, 2023 at 5:18 PM Thomas Monjalon
> <thomas@monjalon.net> wrote:
> > > >
> > > > > 03/04/2023 16:56, Juraj Linkeš:
> > > > > > On Mon, Apr 3, 2023 at 2:33 PM Thomas Monjalon
> > > > > > <thomas@monjalon.net>
> > > > > wrote:
> > > > > >
> > > > > > > 03/04/2023 13:46, Juraj Linkeš:
> > > > > > > > Replace pexpect with Fabric.
> > > > > > >
> > > > > > > You should squash these lines with the move to Fabric.
> > > > > > >
> > > > > > > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > > > > > > ---
> > > > > > > >  dts/poetry.lock    | 553
> > > > > +++++++++++++++++++++++++++++++++++++++------
> > > > > > >
> > > > > > > Do we really need *all* these lines?
> > > > > > > I see a lot of lines about Windows and MacOSX which are not
> > > > > > > supported
> > > > > in
> > > > > > > DTS.
> > > > > > > It is so long that it looks impossible to review.
> > > > > > >
> > > > > > >
> > > > > > This is a generated file and doesn't need to be reviewed.
> > > > >
> > > > > In general, I don't like storing generated files.
> > > > >
> > > >
> > > > Me neither, but this one is specifically designed to be stored in
> > > > a
> > > > repository:
> > > > https://python-poetry.org/docs/basic-usage/#commit-your-poetrylock
> > > > -file-to-version-control
> > > >
> > > >
> > > > >
> > > > > > I separated the
> > > > > > dependencies part so that the code part is easier to review.
> > > > > > If you
> > > > > want, I
> > > > > > can squash the two commits.
> > > > >
> > > > > What happens if we manually remove the useless lines?
> > > > >
> > > > >
> > > > The lock file is there so that everyone installs exactly the same
> > > > versions of dependencies. We can specify the versions of
> > > > dependencies in pyproject.toml, but we won't control the versions
> > > > of dependencies of dependencies this way. If we remove the changes
> > > > to the lock file, then we won't be storing tested versions,
> > > > everyone would be using slightly different versions and we may
> > > > potentially need to address versioning issues in the future - best to prevent
> that with a lock file.
> > >
> > > You didn't answer about removing the usuless lines, like unneeded Windows
> support.
> > >
> >
> > Do you mean the list of files from macos and windows? I tried removing
> > those from mypy and testing it and it looks like it didn't have an
> > impact, but I don't know the inner workings of poetry and the lock
> > file to test it properly (i.e. to rule out any breakages). What would
> > be the reason for removing those? Seems like it has more downsides (we
> > could potentially break something and it's extra work) than updsides
> > (as this is a generated file, I don't really see any).
> 
> Yes this is what I mean.
> Any other opinion?
> 
If it is a generated file, there might be an expectation from the tool that the file is not changed. It would be good to understand this.

Since it is a generated file, should we generate this during DTS run time rather than storing a generated file?

>