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

[Thomas Monjalon <thomas@monjalon.net>]

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?