Re: [pgadmin-hackers] [pgAdmin4] [PATCH] History Tab rewrite in React
От | Dave Page |
---|---|
Тема | Re: [pgadmin-hackers] [pgAdmin4] [PATCH] History Tab rewrite in React |
Дата | |
Msg-id | CA+OCxoxtxBO-mWfUokBu1WG8VH4E6MrHj16mrKT=L0svYvKfGg@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: [pgadmin-hackers] [pgAdmin4] [PATCH] History Tab rewrite in React (George Gelashvili <ggelashvili@pivotal.io>) |
Ответы |
Re: [pgadmin-hackers] [pgAdmin4] [PATCH] History Tab rewrite in React
|
Список | pgadmin-hackers |
Hi OK, so Ashesh and I spend much of the morning on this. Patch 01 - Applied. Patch 02: - karma.conf.js wouldn't patch; I've manually handled that. - test-main.js wouldn't patch. The diff looked like it was trying to empty it; I have removed it instead. - The imports in pgAdmin4.py need to be made after the app root is added to the path. - The JS bundler should be in pgadmin/utils, not pgadmin/tools (which is intended for plugin modules) - The tests were failing following some changes Ashesh pushed earlier to add a client-side url_for function. - pgAdmin4.py was attempting to run the bundler on every startup. I've wrapped those called in "if config.DEBUG:" conditionals, as typically an installation for an end-user will be in a read-only directory. We've fixed all of that in the attached patch. I'm not sure why it's so much bigger than yours. The following issues are outstanding; please take a look at them: - There is no update to the Windows installer generation code (needed in 2 places unfortunately; Make.bat and Make-MinGW.bat). - The updates to the other packages call "yarn run webpacker" which is an undefined target. I haven't looked at patch 03 yet, but Ashesh did tell me it won't apply for him. Patch 4 is also untested at this stage. If the issues above can be fixed, we can get patch 2 applied then move on from there. I'll hold off on Harshal's patch for the Query Tool's load on demand to give you a chance to get this done. Thanks. On Sat, Jun 10, 2017 at 2:52 AM, George Gelashvili <ggelashvili@pivotal.io> wrote: > Hi Dave, > > Our patch touches code also changed by the patches that were recently > committed. > That's likely what's causing this issue. We've rebased on top of the new > state of master. > > We had initially kept the yarn.lock .gitignored, but ran into an issue > rather early on where an upgraded dependency introduced a regression. > Checking-in the yarn.lock provides the "know your dependency version" > benefit of vendorizing code without vendorization's drawback of having to > manually manage your dependencies. > > It is safe to delete a yarn.lock before applying a patch, as you are > authoring master. It provides a history of the versions of each dependency > that were working at the point in time of the commit. By contrast, > package.json provides approximate versions and leaves room for > fixes/improvements by the dependency authors to be pulled in as they become > available. > > To run linter and tests: > > The tasks that Grunt used to manage are now defined as a set of scripts in > the package.json > After applying the patches--which may require deleting yarn.lock for the > first patch--you should cd web && yarn install > > Then yarn test will run the linter, jasmine specs, and python tests > including feature tests, in that order, exiting early if there are > failures/errors. > At the moment, the CheckForViewData test is failing on master as well as in > each of these patches; that should be resolved as RM2477. > > Thanks! > George and Matt > > > On Thu, Jun 8, 2017 at 9:15 AM, Dave Page <dpage@pgadmin.org> wrote: >> >> Hi George >> >> On Wed, Jun 7, 2017 at 10:21 PM, George Gelashvili >> <ggelashvili@pivotal.io> wrote: >> > Hi Dave, >> > >> > I split the linting out into an intermediate commit, and rebased on top >> > of >> > master. >> >> Unfortunately, it still doesn't apply: >> >> error: patch failed: web/regression/javascript/test-main.js:1 >> error: removal patch leaves file contents >> error: web/regression/javascript/test-main.js: patch does not apply >> Checking patch web/regression/requirements.txt... >> Checking patch web/webpack.config.js... >> Checking patch web/webpack.test.config.js... >> Checking patch web/yarn.lock... >> error: web/yarn.lock: already exists in working directory >> Applied patch .gitignore cleanly. >> Applied patch Make.bat cleanly. >> Applied patch README cleanly. >> Applied patch pkg/mac/build.sh cleanly. >> Applied patch pkg/pip/build.sh cleanly. >> Applied patch pkg/src/build.sh cleanly. >> Applied patch web/.eslintrc.js cleanly. >> Applied patch web/karma.conf.js cleanly. >> Applied patch web/package.json cleanly. >> Applied patch web/pgAdmin4.py cleanly. >> Applied patch web/pgadmin/static/jsx/components.jsx cleanly. >> Applied patch web/pgadmin/tools/javascript/__init__.py cleanly. >> Applied patch web/pgadmin/tools/javascript/javascript_bundler.py cleanly. >> Applied patch web/pgadmin/tools/javascript/tests/__init__.py cleanly. >> Applied patch >> web/pgadmin/tools/javascript/tests/test_javascript_bundler.py >> cleanly. >> Applied patch web/regression/README cleanly. >> Applied patch >> web/regression/javascript/jasmine_capture_warnings_beforeall.js >> cleanly. >> Applied patch web/regression/requirements.txt cleanly. >> Applied patch web/webpack.config.js cleanly. >> Applied patch web/webpack.test.config.js cleanly. >> >> The second (lint update) patch is even worse, with significant number >> change that just don't want to apply. >> >> Clearly yarn.lock needs to be removed from there repo. >> >> Once I can apply a version of this, how should I be running the linter >> and the unit tests? >> >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company > > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Вложения
В списке pgadmin-hackers по дате отправления:
Предыдущее
От: Surinder KumarДата:
Сообщение: [pgadmin-hackers] [pgAdmin4][Patch][RM_ 2477]: New Line text edit pop up renders offpage when the size of the grid exceeds the size of the window
Следующее
От: Dave PageДата:
Сообщение: [pgadmin-hackers] pgAdmin 4 commit: Fix the PostGIS Datatypes in SQL tab,Create / Update