Re: ESLINT: On pgAdmin static javascripts

From: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
To: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>, Fahar Abbas <fahar(dot)abbas(at)enterprisedb(dot)com>
Cc: Dave Page <dpage(at)pgadmin(dot)org>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: ESLINT: On pgAdmin static javascripts
Date: 2018-01-11 05:46:55
Message-ID: CAG7mmozCS6gwgxn5reX25oYMjG2DoOFg2gCp0oXQJ8Egj_qC7g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Murtuza,

On Tue, Jan 9, 2018 at 4:08 PM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:

> All patches looks good to me except one issue as shown in screenshot which
> I observed while opening Preferences dialog subsequently.
>
I was able to reproduce the issue on 'master' branch without these patches
too.
I have fixed the same issue on 'WEBPACK_DEVEL2' branch.

Dependent libraries are using different versions of jQuery, and hence
loading two differct jQuery in the webpack.
And, that's causing the issue.

We can take of the issue in a separate patch.

-- Thanks, Ashesh

>
> --
> ​ Murtuza​
>
> On Tue, Jan 9, 2018 at 1:19 PM, Ashesh Vashi <
> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>>
>>
>> On Tue, Jan 9, 2018 at 12:28 AM, Ashesh Vashi <
>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> Please try rest of the patches...
>>> They are independent of each other.
>>>
>>> -- Thanks, Ashesh
>>>
>>> On Jan 8, 2018 23:45, "Murtuza Zabuawala" <murtuza(dot)zabuawala(at)enterprised
>>> b.com> wrote:
>>>
>>>> ​Hi Ashesh,
>>>>
>>>> I'm not able to apply patch :(
>>>>
>>>> murtuza(at)debian:~/projects/pgadmin4$ git apply ~/Desktop/*.patch
>>>> error: patch failed: web/pgadmin/misc/file_manager/
>>>> static/js/utility.js:10
>>>> error: web/pgadmin/misc/file_manager/static/js/utility.js: patch does
>>>> not apply
>>>>
>>>> murtuza(at)debain:~/projects/pgadmin4$ git status | grep '\.rej' | wc -l
>>>> 55
>>>>
>>> Please find the updated patch for "Browser-specific javascript files",
>> which was having diff for 'web/pgadmin/misc/file_manager/
>> static/js/utility.js'.
>>
>> --
>>
>> Thanks & Regards,
>>
>> Ashesh Vashi
>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>> <http://www.enterprisedb.com/>
>>
>>
>> *http://www.linkedin.com/in/asheshvashi*
>> <http://www.linkedin.com/in/asheshvashi>
>>
>>>
>>>> --
>>>> Regards,
>>>> Murtuza Zabuawala
>>>> EnterpriseDB: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>>
>>>> On Mon, Jan 8, 2018 at 9:11 PM, Ashesh Vashi <
>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>>
>>>>> On Mon, Jan 8, 2018 at 8:52 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>> Hi
>>>>>>
>>>>>> On Mon, Jan 8, 2018 at 3:18 PM, Ashesh Vashi <
>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> On Mon, Jan 8, 2018 at 8:31 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> I think these are impossibly big to properly review by hand,
>>>>>>>> especially where much of them are whitespace changes.
>>>>>>>>
>>>>>>> I ran both jasmine test framework, and feature tests.
>>>>>>> Both are running fine.
>>>>>>>
>>>>>>>>
>>>>>>>> I think the best option is to check the regression tests all pass,
>>>>>>>> do some manual testing yourself, and then commit.
>>>>>>>>
>>>>>>> Done some manual testing.
>>>>>>> It would be helpful, If somebody can help with some more testing,
>>>>>>> which I may have missed.
>>>>>>>
>>>>>>
>>>>>> Please ask one of the team (other than me) :-)
>>>>>>
>>>>> :-)
>>>>>
>>>>> -- Thanks, Ashesh
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> -- Thanks, Ashesh
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> On Mon, Jan 8, 2018 at 1:55 PM, Ashesh Vashi <
>>>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Hi Dave/Team,
>>>>>>>>>
>>>>>>>>> I found many problems while 'eslint' utility on all the static
>>>>>>>>> javascript files of pgAdmin 4.
>>>>>>>>> i.e. <pgAdmin4_path>/web/node_module/.bin/eslint <file_name>
>>>>>>>>>
>>>>>>>>> I had fixed all errors reported by eslint for those files.
>>>>>>>>>
>>>>>>>>> I have created six patches for ease of maintenance, as
>>>>>>>>> possibilities of conflicting with other patches.
>>>>>>>>> * Browser nodes javascript files
>>>>>>>>> * Browser specific javascript files
>>>>>>>>> * pgAdmin common javascript files
>>>>>>>>> * SQLEditor/DataGrid javascript files
>>>>>>>>> * Tools javascript files
>>>>>>>>> * Miscellaneous modules javascript files
>>>>>>>>>
>>>>>>>>> All patches are independent of each other.
>>>>>>>>>
>>>>>>>>> Please review it, and share your opinion.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>>
>>>>>>>>> Thanks & Regards,
>>>>>>>>>
>>>>>>>>> Ashesh Vashi
>>>>>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>>>>>> <http://www.enterprisedb.com>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> *http://www.linkedin.com/in/asheshvashi*
>>>>>>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Dave Page
>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>> Twitter: @pgsnake
>>>>>>>>
>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Dave Page
>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>> Twitter: @pgsnake
>>>>>>
>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>> The Enterprise PostgreSQL Company
>>>>>>
>>>>>
>>>>>
>>>>
>>
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2018-01-11 06:28:42 pgAdmin 4 commit: Enable webpackaging the pgadmin javascript files in d
Previous Message pgAdmin 4 Jenkins 2018-01-10 11:45:20 Jenkins build is back to normal : pgadmin4-master-python33 #437