Hi On Wed, Jun 26, 2019 at 8:20 AM Yosry Muhammad <yosry...@gmail.com> wrote:
> Hi, > > On Wed, Jun 26, 2019 at 1:01 PM Dave Page <dp...@pgadmin.org> 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! >> >> > I will disable the generated queries for now, then for the next patch I > will work on (optionally) including them (mogrified). Should I send a patch > with the completed work then start working on the tests and documentation > (for it to get reviewed)? or wait until the patch is complete with tests > and documentation? > We always want to commit the docs and tests along with the code so we don't get in a situation where they later get missed or omitted. Thanks. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
