From: | George Gelashvili <ggelashvili(at)pivotal(dot)io> |
---|---|
To: | Dave Page <dpage(at)pgadmin(dot)org> |
Cc: | Sarah McAlear <smcalear(at)pivotal(dot)io>, Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>, Matthew Kleiman <mkleiman(at)pivotal(dot)io>, Joao Pedro De Almeida Pereira <jdealmeidapereira(at)pivotal(dot)io>, Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org> |
Subject: | Re: [pgAdmin4] [PATCH] History Tab rewrite in React |
Date: | 2017-06-10 01:52:12 |
Message-ID: | CAHowoHZFkDQrMO0-e2gx3ffo3kB56Qwb3jT2S1M7Sc8Bpo0Usw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
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(at)pgadmin(dot)org> wrote:
> Hi George
>
> On Wed, Jun 7, 2017 at 10:21 PM, George Gelashvili
> <ggelashvili(at)pivotal(dot)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
>
Attachment | Content-Type | Size |
---|---|---|
01-start-tracking-yarn-lock.diff | text/plain | 317 bytes |
02-bring-react-into-the-codebase.diff | text/plain | 319.1 KB |
03-lint.diff | text/plain | 68.8 KB |
04-create-query-history-list.diff | text/plain | 43.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Dave Page | 2017-06-11 12:56:55 | pgAdmin 4 commit: Couple of minor performance tweaks. Tests on my Windo |
Previous Message | Shirley Wang | 2017-06-09 13:05:50 | Re: [patch] Changing the ACI tree font to Helvetica |