public inbox for [email protected]  

help / color / mirror / Atom feed

From: Murtuza Zabuawala <[email protected]>
To: Joao De Almeida Pereira <[email protected]>
Cc: Rahul Soshte <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Patch][Feature #1998] Appends .sql if extension not given when using 'save' or 'save as' feature
Date: Fri, 30 Mar 2018 16:10:47 +0530
Message-ID: <CAKKotZR_c5xJ+U-4bYQJ0DJHvDPxPK1aWOi7b7APFhNzpr1F9w@mail.gmail.com> (raw)
In-Reply-To: <CAE+jjam7ks2X=x5WoEp7WEn4tLNzw8Gzu+Mqx1sUm0OUu4Md-A@mail.gmail.com>
References: <CAKyzeV1fMB6rPMF+LCMY7B6_2-j7DZfmh307dBFemtR=4taX6g@mail.gmail.com>
	<CAE+jjam7ks2X=x5WoEp7WEn4tLNzw8Gzu+Mqx1sUm0OUu4Md-A@mail.gmail.com>

On Thu, Mar 29, 2018 at 11:45 PM, Joao De Almeida Pereira <
[email protected]> wrote:

> Hi Rahul,
> I see you extracted some code, that is a pretty good move :D
>
> We run the patch through the testing pipeline and everything is green GJ :D
> Also tested the functionality by hand and looks like it is working except
> for "add the .sql extension when format is set to SQL." if you set it to
> All Files  the extension is also added. Not sure if this is a big deal or
> not. Lets see what other people think.
>
​Yes, I also think it should append .sql only if the sql extension is
selected and user has not provided extension.​

​Let say If I want to save the file with .txt extension then I can use All
Files. ​


> Codewise here are some of my comments:
> . You added the yarn-error.log file and a migration to the patch doesn't
> look intentional. Can you please remove them?
> . Also in the patch there are 2 file (moc_LogWindow.cpp and
> ui_LogWindow.h) that look like they do not belong to the patch (Did you
> rebase your branch before trying to create the patch?
>
> The test file: test_save_query_to_file.py is empty, it is missing some
> tests there.
>
> As a convention we user lower case names for functions and UpperCase for
> class
>
> Please, regenerate the patch following my previous comments.
>
> Thanks
> Joao
>
> On Thu, Mar 29, 2018 at 12:54 PM Rahul Soshte <[email protected]>
> wrote:
>
>> Hi,
>> When using save or save as feature if .sql is not provided this Patch
>> appends it.
>> as clearly mentioned in this link.
>>
>> https://redmine.postgresql.org/issues/1998
>>
>> I have ran pep8,regression and Jasmine tests too.
>>
>> I have primarily changed these files
>>      web/pgadmin/tools/sqleditor/__init__.py
>>      web/pgadmin/tools/sqleditor/static/js/sqleditor.js
>>      web/pgadmin/tools/sqleditor/utils/save_query_to_file.py
>>
>>
>> Regards,
>> Rahul Soshte (Hunter)
>>
>>

view thread (20+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: [pgAdmin4][Patch][Feature #1998] Appends .sql if extension not given when using 'save' or 'save as' feature
  In-Reply-To: <CAKKotZR_c5xJ+U-4bYQJ0DJHvDPxPK1aWOi7b7APFhNzpr1F9w@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox