Em qui., 15 de jul. de 2021 às 09:45, David Rowley <dgrowle...@gmail.com>
escreveu:

> On Thu, 15 Jul 2021 at 23:38, Aleksander Alekseev
> <aleksan...@timescale.com> wrote:
> > I'm updating the status to "Ready for Committer".
>
> I think that might be a bit premature.  I can't quite see how changing
> the pids List to a const List makes any sense, especially when the
> code goes and calls lappend_int() on it to assign it some different
> value.
>
> There are also problems in BackendPidGetProcWithLock around consts.
>
> Much of this patch kinda feels like another one of those "I've got a
> fancy new static analyzer" patches.  Unfortunately, it just introduces
> a bunch of compiler warnings as a result of the changes it makes.
>
> I'd suggest splitting each portion of the patch out into parts related
> to what it aims to achieve.  For example,  it looks like there's some
> renaming going on to remove a local variable from shadowing a function
> parameter.  Yet the patch is claiming performance improvements.  I
> don't see how that part relates to performance. The changes to
> ProcArrayClearTransaction() seem also unrelated to performance.
>
> I'm not sure what the point of changing things like for (int i =0...
> to move the variable declaration somewhere else is about.  That just
> seems like needless stylistic changes that achieve nothing but more
> headaches for committers doing backpatching work.
>
> I'd say if this patch wants to be taken seriously it better decide
> what it's purpose is, because to me it looks just like a jumble of
> random changes that have no clear purpose.
>
> I'm going to set this back to waiting on author.
>
I understood.
I will try to address all concerns in the new version.

regards,
Ranier Vilela

Reply via email to