Re: [PATCH] scripts: avoid redundant re-reading makefiles.

[Alexander Pantyukhin <apantykhin@gmail.com>]

Hi Miguel.
Please see my comments below.

чт, 5 янв. 2023 г. в 22:44, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>:
>
> Hi Alex,
>
> The commit message is looking better, thank you. Please see comments
> inline below.
>
> On Wed, Jan 4, 2023 at 5:04 PM Alexander Pantyukhin
> <apantykhin@gmail.com> wrote:
> >
> > The main goal of the patch is avoid redundant re-reading makefiles
> > if these were read before. This is performance improvement.
>
> Like it was mentioned in v1, could you please give a rough estimate
> about what is the improvement from old to new? (in particular, without
> the other changes). That way, others can get an idea whether the code
> complexity is worth it or not.

I checked files now. It seems only one place in /samples  has 2 files
with 1 makefile in the parent folder. So it looks like the patch is
untimely for now.

>
> > Additional code improvements are using 'with' when read file and check
> > if file exists.
>
> Note that others submitted an improvement for the missing file case as
> we discussed in v1, so it would be better if they submitted their own
> patches for those. I am Cc'ing them so that they have a chance to be
> aware of this patch and potentially submit theirs.
>
> In any case, their approach seems more idiomatic for Python [1], so I
> think we should do it that way.
>
> Finally, the title of this commit should probably start with the
> "scripts: generate_rust_analyzer: " prefix, otherwise it is a bit too
> broad.

I see, thanks. Still have a question about using "with". Is it reasonable?

>
> Thanks!
>
> Cheers,
> Miguel
>
> [1] https://docs.python.org/3.11/glossary.html#term-EAFP

Thank you!
Best, Alex.