Re: [pgAdmin4][Patch][Feature #1998] Appends .sql if extension not given when using 'save' or 'save as' feature

On Thu, Mar 29, 2018 at 11:45 PM, Joao De Almeida Pereira <
jdealmeidapereira(at)pivotal(dot)io> 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 <rahulsoshte360(at)gmail(dot)com>
> 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)
>>
>>