Re: [GSoC] Finalized First Patch

Hi

On Fri, Jul 5, 2019 at 6:28 AM Yosry Muhammad <yosrym93(at)gmail(dot)com> wrote:

> - The patch won't apply with "git apply" and only partially applies with
>> patch -p0. Please "git add" all your changes and new files in your repo,
>> and then run "git diff --cached --binary", which should create a usable
>> patch. You can then un-stage your changes.
>>
>>
> That was due to new image. I made an applicable one with the below
> modifications, please find it attached.
>

Thanks!

>
>
>> - execute_query_utils.py is somewhat lacking in file header and any
>> comments.
>>
>>
> Added.
>
> - There is commented-out code in sqleditor.js
>>
>
> This is the call that adds queries that are generated by pgAdmin to the
> query history. I commented it instead of removing it as I will add it later
> with some modifications when I add the checkbox.
>

Sure, but we won't commit commented-out code. It just makes things messy
until such time as it gets used (or more often, does not).

> - On reflection, I don't think the "Data saved successfully, you still
>> need to commit changes to the database." is prominent enough - in testing,
>> I found it very easy to miss. That might also be compounded by the fact
>> that "Alert on uncommitted transactions?" doesn't seem to be working for
>> me. I get the "Save text" prompt to save the query text, but if I say no to
>> that, the Query Tool instance closes with no further warning, despite
>> having a transaction in progress.
>>
>
> I made a separate notification for the uncommitted save to make it more
> visible, check it out.
>

That's definitely clearer now. It avoids the "blindness" caused by the fact
that you always get the green message.

> I tried the scenario you provided and the uncommitted alert worked fine,
> could you please try again and tell me the exact scenario where that
> happened?
>

Hmm, it's working fine for me today too. Definitely wasn't yesterday though!

I'm going to make some minor tweaks to the wording of the docs before I
commit (as well as removing the commented-out code), but I think this is
good to go, once it's had another review. Khushboo, please take a look as
soon as you can.

Thanks!

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company