Re: [pgadmin-hackers] [pgAdmin4] [PATCH] History Tab rewrite in React

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