Re: [GSoC] Finalized First Patch

Поиск
Список
Период
Сортировка
От Dave Page
Тема Re: [GSoC] Finalized First Patch
Дата
Msg-id CA+OCxox43r3qH9YNt_gT7TT3s+DkROvf1pvvAKzg663xHzT5VQ@mail.gmail.com
обсуждение исходный текст
Ответ на [GSoC] Finalized First Patch  (Yosry Muhammad <yosrym93@gmail.com>)
Ответы Re: [GSoC] Finalized First Patch  (Yosry Muhammad <yosrym93@gmail.com>)
Re: [GSoC] Finalized First Patch  (Khushboo Vashi <khushboo.vashi@enterprisedb.com>)
Re: [GSoC] Finalized First Patch  (Khushboo Vashi <khushboo.vashi@enterprisedb.com>)
Список pgadmin-hackers
Hi

On Fri, Jul 5, 2019 at 6:28 AM Yosry Muhammad <yosrym93@gmail.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
Вложения

В списке pgadmin-hackers по дате отправления:

Предыдущее
От: Dave Page
Дата:
Сообщение: Re: [pgAdmin][RE-SQL] Foregin Data Wrappers
Следующее
От: Yosry Muhammad
Дата:
Сообщение: Re: [GSoC] Finalized First Patch