Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hg5gd-00056D-Mq for pgadmin-hackers@arkaria.postgresql.org; Wed, 26 Jun 2019 11:02:00 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hg5gc-0002bi-DQ for pgadmin-hackers@arkaria.postgresql.org; Wed, 26 Jun 2019 11:01:58 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hg5gb-0002ar-Tc for pgadmin-hackers@lists.postgresql.org; Wed, 26 Jun 2019 11:01:58 +0000 Received: from mail-wr1-x443.google.com ([2a00:1450:4864:20::443]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hg5gY-0000Nw-FX for pgadmin-hackers@postgresql.org; Wed, 26 Jun 2019 11:01:56 +0000 Received: by mail-wr1-x443.google.com with SMTP id n9so2248152wru.0 for ; Wed, 26 Jun 2019 04:01:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=/GB0bySzmP6IIwRqeElO3NQrW8S6geNjGo7bBfPnoeA=; b=H0eynZeNw77oGOyXtkCdz59mwpWayJBYsic9wfx2cvAIKRr4F+205L8OuhDnKhbirF QroIzouDRvGeGZ/y4OiqGsbEG+Texu5XOC+7Mf0mALsaNWNjOtYU9q8/AuVVrfI4S0Lb sJnp1Xeqnjx7M9lsEJkox0ZvYSpR9sVLkP7HgQe0fR0qOdywdgmkapDph6iO4XDO9Lti NNi6XjdD/pMNrPINWJhV34F0fEaSlLqWbF/31xpZCgpNJK6s5PB3OdUm0K8HOfGFlJLR tQBBBbpICieWW/9xWncsh961orFoqRG4ncJRKHG7X159RUi+If3lZOcep3FwQGLELY0v THQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=/GB0bySzmP6IIwRqeElO3NQrW8S6geNjGo7bBfPnoeA=; b=tLITP6+hAal+YJCV8tsmQF187pWNuQwIVF8g+yFk7GKVuJv/dX+/nOUj7ZTSKiMDyz kUaeIBqssqucXNZX2pRGgcoUwEYFRwJTqNoXbMOPZrst8UPb8XSbDpGvUzd4MQB69Wy5 L87vESQPs7PzEYROAXCPkz5vJkdiPldIX+RQWlJEllBZyLxV6JEq/xIzUrm2CJMxrmSl Mhr8mmGjIX/+VQSQrS0qLYplm6asGTvoXEvMNZ5m9YAyQmpNZmJpFYd7/KQ5dvnzfDXR gUAaAX3QwHzlOnlbipb/Ao9fWbpVBFVJi4AxYQCZjgd4lyN73i5hFRp43loPyIGXu8us LtFA== X-Gm-Message-State: APjAAAVHuUyh0CZp4JD5wNGRYXOadStETQB3vpC9XpO3oZn/oAPeBhx5 NcjyOxtP0p4UGmV6TP/1ZkLMtV2LfF7tBZbT5iLdZQ== X-Google-Smtp-Source: APXvYqyZ3fCkL4+qptZ8kHERvFCtkcG/f039E4nTvpKHTX2ubaBTkOXvF6KTzmkthnZG12lcCb+Qc4HSDV1YY+3RrWE= X-Received: by 2002:a05:6000:106:: with SMTP id o6mr3355450wrx.4.1561546912854; Wed, 26 Jun 2019 04:01:52 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Dave Page Date: Wed, 26 Jun 2019 07:01:40 -0400 Message-ID: Subject: Re: [GSoC][Patch] Automatic Mode Detection V1 To: Yosry Muhammad Cc: Aditya Toshniwal , pgadmin-hackers Content-Type: multipart/alternative; boundary="0000000000000a8647058c37f941" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --0000000000000a8647058c37f941 Content-Type: text/plain; charset="UTF-8" Hi On Tue, Jun 25, 2019 at 11:20 AM Yosry Muhammad wrote: > > > On Tue, Jun 25, 2019 at 1:09 PM Dave Page wrote: > >> >> - What else is missing from this patch to make it applicable ? I would >>>> like to produce a release-ready patch if possible. If so, I can continue >>>> working on the project on following patches, I just want to know what is >>>> the minimum amount of work needed to make this patch release-ready >>>> (especially that changes are being made in the master branch that require >>>> me to re-edit parts of the code that I have written before to keep things >>>> in-sync). >>>> >>> @Dave Page is the right person to answer this. >>> >> >> It needs: >> >> - A code complete feature (or infrastructure/refactoring ready for a >> feature), that is acceptable to us. Seems like this is 90% there for an >> initial commit. >> - Documentation updates. >> - Tests for the new feature to ensure it works without needing manual >> testing. >> - To pass all existing tests (which may be modified if appropriate). >> >> > > Could you tell me what is missing from this patch (in terms of features - > other than tests) to be acceptable? I will start working on the tests once > the patch is complete. The patch passes all the existing tests except for 3 > feature tests that fail due to a TimeoutException in selenium. I do not > know what this is about I hope Aditya will help me with it. > Here are the issues I think should be fixed first: - I think the Save button should be moved to the left of the Find button. It makes more sense to be near the Save Query button. - Umm, that's about it, bar the history issue. The quick fix there might be to hide the internal queries for now as previously discussed, but I do this the checkbox to include them (in their mogrified state) should be included as part of the overall project. Note that I haven't done in-depth testing. Once the patch is committed (and about now is a good time, as we're at the beginning of the release cycle), we'll get our QA guy to see if he can find any issues we've missed. > > Also, do you mean code documentation or documentation for the users? Could > you point me towards the related parts? > User documentation. I would expect at least one screenshot update due to the new button on the toolbar (probably more - please check for others that need updating), as well as updates to at least: editgrid.rst query_tool.rst query_tool_toolbar.rst Great work! -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company --0000000000000a8647058c37f941 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi

On Tue, Jun 25, 2019 at 11:20 AM Yosry Muhammad <= ;yosrym93@gmail.com> wrote:
<= div dir=3D"ltr">

On Tue, Jun 25, 2019 at 1:09 PM Dave Page <dpage@pgadmin.org> w= rote:

- What else is miss= ing from this patch to make it applicable ? I would like to produce a relea= se-ready patch if possible. If so, I can continue working on the project on= following patches, I just want to know what is the minimum amount of work = needed to make this patch release-ready (especially that changes are being = made in the master branch that require me to re-edit parts of the code that= I have written before to keep things in-sync).
@D= ave Page is the right person to answer this.=C2=A0
=

It needs:

- A cod= e complete feature (or infrastructure/refactoring ready for a feature), tha= t is acceptable to us. Seems like this is 90% there for an initial commit.<= /div>
- Documentation updates.
- Tests for the new feature to= ensure it works without needing manual testing.
- To pass all ex= isting tests (which may be modified if appropriate).
=C2=A0
=

Could you tell me what is miss= ing from this patch (in terms of features - other than tests) to be accepta= ble? I will start working on the tests once the patch is complete. The patc= h passes all the existing tests except for 3 feature tests that fail due to= a TimeoutException in selenium. I do not know what this is about I hope Ad= itya will help me with it.

Here are the issues I think should be fixed first:

- I think the Save button should be moved to the left of the Find button= . It makes more sense to be near the Save Query button.

- Umm, that's about it, bar the history issue. The quick fix ther= e might be to hide the internal queries for now as previously discussed, bu= t I do this the checkbox to include them (in their mogrified state) should = be included as part of the overall project.

Note t= hat I haven't done in-depth testing. Once the patch is committed (and a= bout now is a good time, as we're at the beginning of the release cycle= ), we'll get our QA guy to see if he can find any issues we've miss= ed.
=C2=A0

Also, do y= ou mean code documentation or documentation for the users? Could you point = me towards the related parts?

=
User documentation. I would expect at least one screenshot update due = to the new button on the toolbar (probably more - please check for others t= hat need updating), as well as updates to at least:

editgrid.rst
query_tool.rst
query_tool_toolbar.rst=C2= =A0

Great work!

--
<= div dir=3D"ltr" class=3D"gmail_signature">Dave Page
Blog: http://pgsnake.blogspot.com=
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterpr= ise PostgreSQL Company
--0000000000000a8647058c37f941--