Re: Pgadmin4 System Stats Extension Design

Hi Sahil,

Your PR is showing 135 file changes and a lot of commits which shouldn't
have appeared on your PR.
It is very difficult to identify your changes. Can you please check once?

On Fri, Sep 8, 2023 at 1:13 AM Sahil Harpal <sahilharpal1234(at)gmail(dot)com>
wrote:

> Hi Aditya,
>
> Sorry for the delay; I've been a bit busy lately. I have made all the
> requested changes. Could you please review it?
>
> Thanks,
> Sahil
>
> On Mon, 4 Sept 2023 at 11:17, Aditya Toshniwal <
> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>> Hi Sahil,
>>
>> I have replied to the PR.
>>
>> On Sun, Sep 3, 2023 at 2:31 AM Sahil Harpal <sahilharpal1234(at)gmail(dot)com>
>> wrote:
>>
>>> Hi Aditya,
>>>
>>> I have made almost all of the requested changes and pushed the latest
>>> code. I just need a bit of clarification for a couple of suggestions that I
>>> have posted in the reviews.
>>>
>>> Thank you,
>>> Sahil
>>>
>>>
>>> On Thu, 31 Aug 2023 at 17:20, Aditya Toshniwal <
>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Sahil,
>>>>
>>>> OK fine. We will check it later. Not priority. Please also fix the
>>>> review raised on PR.
>>>>
>>>> On Thu, Aug 31, 2023, 16:17 Sahil Harpal <sahilharpal1234(at)gmail(dot)com>
>>>> wrote:
>>>>
>>>>> Hi Aditya,
>>>>> I have made all these changes except the StreamingChart issue. I tried
>>>>> filling an array with null values inside the StreamingChart component while
>>>>> initializing initialState but still the issue is not resolved.
>>>>> I have made following changes:
>>>>>
>>>>>> const initialState = [
>>>>>> Array.from(new Array(xRange).keys()),
>>>>>> ...(data.datasets?.map((d)=>{
>>>>>> let nullValues = new Array(xRange - d.data.length).fill(null);
>>>>>> let ret = [...nullValues, ...d.data];
>>>>>> ret.reverse();
>>>>>> return ret;
>>>>>> })??{}),
>>>>>> ];
>>>>>
>>>>>
>>>>> It works fine if we initialize the data array with the null values but
>>>>> I'm not getting why this is not working.
>>>>>
>>>>> Thank you,
>>>>> Sahil
>>>>>
>>>>>
>>>>> [image: Mailtrack]
>>>>> <https://mailtrack.io?utm_source=gmail&utm_medium=signature&utm_campaign=signaturevirality11&> Sender
>>>>> notified by
>>>>> Mailtrack
>>>>> <https://mailtrack.io?utm_source=gmail&utm_medium=signature&utm_campaign=signaturevirality11&> 31/08/23,
>>>>> 16:14:27
>>>>>
>>>>> On Mon, 28 Aug 2023 at 10:44, Aditya Toshniwal <
>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Sahil,
>>>>>>
>>>>>> I have few observations. You have added separate titles for graphs
>>>>>> and other tabular data. This is inconsistent with existing UI.
>>>>>> For example,
>>>>>> [image: image.png]
>>>>>> like here:
>>>>>> [image: image.png]
>>>>>>
>>>>>> [image: Screenshot 2023-08-28 at 10.28.59 AM.png]
>>>>>> like here:
>>>>>> [image: image.png]
>>>>>>
>>>>>> [image: image.png]
>>>>>>
>>>>>> [image: image.png]
>>>>>>
>>>>>> The dashboard goes blank when I change refresh rates.
>>>>>>
>>>>>> [image: image.png]
>>>>>>
>>>>>> And regarding filling with nulls to fix the reversing issue of graph
>>>>>> - you can do it in StreamingChart itself when setting initialState var, as
>>>>>> it is StreamingChart's responsibility to do it.
>>>>>> Next review will be on the PR directly.
>>>>>>
>>>>>>
>>>>>> On Sun, Aug 27, 2023 at 5:58 PM Sahil Harpal <
>>>>>> sahilharpal1234(at)gmail(dot)com> wrote:
>>>>>>
>>>>>>> Hello everyone,
>>>>>>>
>>>>>>> I have raised the PR
>>>>>>> <https://github.com/pgadmin-org/pgadmin4/pull/6721>.
>>>>>>> I would like to request you all to review the changes and provide
>>>>>>> your valuable feedback. Your insights and suggestions would be invaluable
>>>>>>> in ensuring the quality and accuracy of the codebase.
>>>>>>>
>>>>>>> Thank you,
>>>>>>> Sahil
>>>>>>>
>>>>>>> On Sun, 27 Aug 2023 at 07:13, Khushboo Vashi <
>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Sat, 26 Aug 2023, 11:36 Sahil Harpal, <sahilharpal1234(at)gmail(dot)com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi Khushboo,
>>>>>>>>>
>>>>>>>>> On Mon, 21 Aug 2023 at 10:03, Khushboo Vashi <
>>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Sahil, once the issues get resolved, please raise the PR and we
>>>>>>>>>> will do the final review there.
>>>>>>>>>>
>>>>>>>>> Could you please tell me to which branch I should raise the PR?
>>>>>>>>>
>>>>>>>> Master branch
>>>>>>>>
>>>>>>>>> Also, should I remove the code responsible for the static
>>>>>>>>> DonutChart of process information?
>>>>>>>>>
>>>>>>>>> Thank you,
>>>>>>>>> Sahil
>>>>>>>>>
>>>>>>>>>>
>>>>>>
>>>>>> --
>>>>>> Thanks,
>>>>>> Aditya Toshniwal
>>>>>> pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*
>>>>>> <https://www.enterprisedb.com/>
>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>
>>>>>
>>
>> --
>> Thanks,
>> Aditya Toshniwal
>> pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*
>> <https://www.enterprisedb.com/>
>> "Don't Complain about Heat, Plant a TREE"
>>
>

--
Thanks,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*
<https://www.enterprisedb.com/>
"Don't Complain about Heat, Plant a TREE"