Hi Nikhil Following are the review comments:
*GUI specific*: - We need a panel icon for PSQL like query tool, we can also add that on the browser tree toolbar. - PSQL Tool menu should be visible for all the child nodes of the database node. Follow the same as Query Tool. - PSQL tab title should be only database server name as the user can change the database/user from PSQL command, so it's been difficult to update the tab title. - PSQL connection is still open even if we disconnect the database server from the browser tree. *Code specific:* - Remove an extra space from requirements.txt and package.json - Documentation needs to be updated to let the user know from where the PSQL tool will open and on which node it is applicable. - psql/__init__.py check there are so many unused imports please remove them. - We are not using cheroot so it should be removed from requirements.txt and also remove the import statement from pgAdmin4.py - Test cases are showing successful but actually, there are some routing errors please check. On Mon, May 10, 2021 at 7:32 PM Nikhil Mohite < nikhil.moh...@enterprisedb.com> wrote: > Hi Dave/ Team, > > PFA updated patch, sorry for the inconvenience, while cleanup I removed > the unwanted libraries but forgot to remove the code related to them. > > On Mon, May 10, 2021 at 7:10 PM Dave Page <dp...@pgadmin.org> wrote: > >> Hi >> >> On Mon, May 10, 2021 at 1:45 PM Nikhil Mohite < >> nikhil.moh...@enterprisedb.com> wrote: >> >>> Hi Hackers, >>> >>> Please find the attached patch for RM-2341 >>> <https://redmine.postgresql.org/issues/2341>: Add Menu option for >>> starting PSQL. >>> 1. Added new Option PSQL Tool in Tools menu. >>> 2. Added the same option for Server and Database nodes from the tree >>> view. >>> >> >> Unfortunately there's a trailing comma in package.json that makes it >> invalid. If I fix that, then I get the error below, so I'm guessing the >> intention was to actually include another package there? >> >> ERROR in ./pgadmin/tools/psql/static/js/psql_module.js 23:50-82 >> Module not found: Error: Can't resolve 'local-echo-controller' in >> '/Users/dpage/git/pgadmin4/web/pgadmin/tools/psql/static/js' >> resolve 'local-echo-controller' in >> '/Users/dpage/git/pgadmin4/web/pgadmin/tools/psql/static/js' >> Parsed request is a module >> using description file: /Users/dpage/git/pgadmin4/web/package.json >> (relative path: ./pgadmin/tools/psql/static/js) >> aliased with mapping 'local-echo-controller': >> '/Users/dpage/git/pgadmin4/web/node_modules/local-echo' to >> '/Users/dpage/git/pgadmin4/web/node_modules/local-echo' >> using description file: /Users/dpage/git/pgadmin4/web/package.json >> (relative path: ./pgadmin/tools/psql/static/js) >> Field 'browser' doesn't contain a valid alias configuration >> root path /Users/dpage/git/pgadmin4/web >> using description file: >> /Users/dpage/git/pgadmin4/web/package.json (relative path: >> ./Users/dpage/git/pgadmin4/web/node_modules/local-echo) >> no extension >> Field 'browser' doesn't contain a valid alias configuration >> >> /Users/dpage/git/pgadmin4/web/Users/dpage/git/pgadmin4/web/node_modules/local-echo >> doesn't exist >> .js >> Field 'browser' doesn't contain a valid alias configuration >> >> /Users/dpage/git/pgadmin4/web/Users/dpage/git/pgadmin4/web/node_modules/local-echo.js >> doesn't exist >> .jsx >> Field 'browser' doesn't contain a valid alias configuration >> >> /Users/dpage/git/pgadmin4/web/Users/dpage/git/pgadmin4/web/node_modules/local-echo.jsx >> doesn't exist >> as directory >> >> /Users/dpage/git/pgadmin4/web/Users/dpage/git/pgadmin4/web/node_modules/local-echo >> doesn't exist >> using description file: >> /Users/dpage/git/pgadmin4/web/package.json (relative path: >> ./node_modules/local-echo) >> no extension >> Field 'browser' doesn't contain a valid alias configuration >> /Users/dpage/git/pgadmin4/web/node_modules/local-echo doesn't >> exist >> .js >> Field 'browser' doesn't contain a valid alias configuration >> /Users/dpage/git/pgadmin4/web/node_modules/local-echo.js >> doesn't exist >> .jsx >> Field 'browser' doesn't contain a valid alias configuration >> /Users/dpage/git/pgadmin4/web/node_modules/local-echo.jsx >> doesn't exist >> as directory >> /Users/dpage/git/pgadmin4/web/node_modules/local-echo doesn't >> exist >> @ ./pgadmin/tools/psql/static/js/index.js 17:19-43 >> >> 2021-05-10 14:38:37: webpack 5.21.2 compiled with 1 error in 60041 ms >> >> -- >> Dave Page >> Blog: https://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EDB: https://www.enterprisedb.com >> > > Regards, > Nikhil Mohite > -- *Thanks & Regards* *Akshay Joshi* *pgAdmin Hacker | Principal Software Architect* *EDB Postgres <http://edbpostgres.com>* *Mobile: +91 976-788-8246*
