Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dHkbG-0004CC-1u for pgadmin-hackers@arkaria.postgresql.org; Mon, 05 Jun 2017 05:30:46 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1dHkbF-0003bv-Bh for pgadmin-hackers@arkaria.postgresql.org; Mon, 05 Jun 2017 05:30:45 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1dHkbE-0003bl-Iy for pgadmin-hackers@postgresql.org; Mon, 05 Jun 2017 05:30:44 +0000 Received: from mail-wr0-x235.google.com ([2a00:1450:400c:c0c::235]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1dHkb9-0005hM-LJ for pgadmin-hackers@postgresql.org; Mon, 05 Jun 2017 05:30:44 +0000 Received: by mail-wr0-x235.google.com with SMTP id g76so32192802wrd.1 for ; Sun, 04 Jun 2017 22:30:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=ajilhIJ3v6ycDn1oqY4Kmuz5XwufWxbX+4g3VFaXXZs=; b=aBPgakQfFXGULqPs7iKTV1gh7VgLEil6KsntQFQAcNflWKdrLeYA/5ck88Yw1qmp/V FplQWcd/jSax3kXULTBkJTpOSaD7EACV9B43ai5PTq407nTw3wgLRpoX9NXBkj970kl2 gAMQnD7tzDJh4Gtz3KwKZofhxHt8kIs/Yc04TvMm/qgs8R4j9F9IWgFjYgVIogVR7rwY eRwnw6uyMrAA1zkeAPFLRqYLZ6jZVggx4IFKvYy8aJ+tf4Ha/PL6bplhQlslav5YUYuT Q3MGqTE1t5NEIH/sRD6bz8j+0pfK7kytlxsgmpnTWoOJTF/bjNoI7p21OlxjqwveNcp5 I7mA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=ajilhIJ3v6ycDn1oqY4Kmuz5XwufWxbX+4g3VFaXXZs=; b=Xev7AHQp+3n7eFksMqGR4w+CtqZi6s9Xqxt8JtA9hNtYRB4/865tdjto07x2WVNZKw amvPYx79RCb/4HU3/53Nvepqg5Gd9uMt1mpRtQQl4nwM4v+jlePO/y9ekTAtYcMqoqsz CX4ifl6VrjvYo7bKODVt+h0xD9I38j4Wguj5Z/UtZ5T+85gR7CDq+nsjVInHXArKGbkc Dp86euhsBQ2hH4vq1OoiSDKmzVzxCzBvnwCjPOWHIt1aKjsaYOJ9gIq2vg+wXw2ulNGy /Ni0ubP1xhgR8MTL9kTpIlprYu3tXbUnOV2OXqWezsK+IP2IkL2Hp4OGMwCLx1c1XNNn nawQ== X-Gm-Message-State: AODbwcBJsPuS9mMReKfFJ/eCjYaZwuojGm5W1RvF5shjUdkXfQj3Ucu4 9Jam1Auq6NBnN+KkUX2JoyrmPQD+6kKW X-Received: by 10.223.147.196 with SMTP id 62mr12745725wrp.107.1496640638096; Sun, 04 Jun 2017 22:30:38 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.19.138 with HTTP; Sun, 4 Jun 2017 22:30:07 -0700 (PDT) In-Reply-To: References: From: Surinder Kumar Date: Mon, 5 Jun 2017 11:00:07 +0530 Message-ID: Subject: Re: [pgAdmin4] [PATCH] History Tab rewrite in React To: Matthew Kleiman Cc: Joao Pedro De Almeida Pereira , Dave Page , Murtuza Zabuawala , pgadmin-hackers Content-Type: multipart/alternative; boundary="94eb2c0d9ece97292005512fcd04" X-Pg-Spam-Score: -1.9 (-) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --94eb2c0d9ece97292005512fcd04 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Review comments: 1) First patch contains changes related to copy row feature which is already in process in another thread. Any reason to add those changes in this patch as this patch is meant to contain infrastructure changes ? 2) Can the changes related to FeatureTest or Jasmine test be separated out in another patch? Thanks, Surinder On Thu, Jun 1, 2017 at 1:59 AM, Matthew Kleiman wrote= : > Hey Surinder, > > Is it possible to run the task listed above by configuring them in >> "scripts" in package.json file and running concurrently ? > > > That would be cleaner than what we proposed. We could use the yarn run > feature to do just what you > are suggesting. It works the same as in npm. > Something like the following: > In package.json - > "scripts": { > "linter": "eslint pgadmin/static/jsx/**/*.jsx > pgadmin/static/js/selection/*.js regression/javascript/**/*.jsx > regression/javascript/**/*.js *.js" > } > > To call it on the command line - > yarn run linter > > - Matt > > On Wed, May 31, 2017 at 12:35 AM, Surinder Kumar < > surinder.kumar@enterprisedb.com> wrote: > >> On Wed, May 31, 2017 at 12:01 AM, Joao Pedro De Almeida Pereira < >> jdealmeidapereira@pivotal.io> wrote: >> >>> The motivation is simple - we want a solution that works for the whole >>>> app, can handle debug vs. release execution, pluggable modules, and >>>> installations in read-only directories. >>> >>> With the current configuration of Grunt, all the requirements you >>> mention are available. >>> The tasks on Grunt should only be run under development or before we ar= e >>> creating the installer. >>> >> >> >>> The installer should pick up only the bundled Javascript and should >>> install it in the correct folders, this way there is no need to rewrite= or >>> process files in read-only directories of the end user machine. >>> >>> Per the other thread on the subject (that Joao suggested continuing >>>> discussion on), Surinder is currently looking into flask-webpack. I >>>> spent some time playing with grunt and some other options last week. >>> >>> We should continue the discussion about flask-webpack or Grunt or any >>> other Build system in the other thread, but we need to have some decisi= on >>> because this patch needs a build system. >>> If we remove Grunt from this patch we will need to execute the followin= g >>> command every time the application run: >>> >>> yarn run eslint pgadmin/static/jsx/**/*.jsx >>> pgadmin/static/js/selection/*.js regression/javascript/**/*.jsx >>> regression/javascript/**/*.js *.js && yarn run webpack -- --config >>> webpack.config.js && python web/pgAdmin4 >>> >>> And to run the jasmine tests: >>> >>> yarn run eslint pgadmin/static/jsx/**/*.jsx >>> pgadmin/static/js/selection/*.js regression/javascript/**/*.jsx >>> regression/javascript/**/*.js *.js && yarn run karma start >>> >>> And to run the feature tests: >>> >>> yarn run eslint pgadmin/static/jsx/**/*.jsx >>> pgadmin/static/js/selection/*.js regression/javascript/**/*.jsx >>> regression/javascript/**/*.js *.js && yarn run webpack -- --config webp= ack.config.js >>> && python runtests.py >>> >>> Is this a solution that is acceptable? >>> >> >> As per my knowledge, Grunt can run the tasks and bundle the JS and CSS >> files (using r.js which is not preferred). But Webpack can not do both. = It >> can only bundle files, it has to be used with either Grunt or Gulp to >> execute tasks. >> >> Is it possible to run the task listed above by configuring them in >> "scripts" in package.json file and running concurrently ? >> >> scripts: { >> "task-name1": "yarn run eslint filename", >> =E2=80=8B =E2=80=8B >> "task-name2": "yarn run karma start", so on... >> } >> >> and run them using following command: >> npm run task-name1 >> >> >>> >>> However; this patch is supposed to be about the history tab rewrite. >>>> Whatever solution we use for webpacking/transpiling/linting/minifying >>>> etc, it should be a standalone change as it's decidedly non-trivial. >>> >>> We split this patch into 2 different commits: >>> 1 - Add React and all the tools needed to work with it, this includes >>> Grunt, Webpack and ESLint >>> 2 - Change the history tab >>> >>> Do you want a single commit for each of the following? >>> React (just adds the React library via yarn) >>> Webpack (adds Webpack library via yarn and creates the >>> webpack.config.js) >>> ESLint (adds ESLint library via yarn, creates .eslintrc config file, >>> and corrects all lint errors that previously existed) >>> Grunt (adds Grunt library via yarn and creates Gruntfile.js config, >>> creating a pipeline of the previous three libraries/tasks) >>> Our change to History >>> >>> Thanks >>> Joao & Matt >>> >>> On Tue, May 30, 2017 at 10:10 AM, Dave Page wrote: >>> >>>> Hi >>>> >>>> On Tue, May 30, 2017 at 2:47 PM, Matthew Kleiman >>>> wrote: >>>> > Hi Dave, >>>> > >>>> > We are currently using the Grunt taskrunner to run the following >>>> tasks: >>>> > >>>> > lint the javascript code >>>> > start javascript tests >>>> > invoke webpack to transpile and bundle js and jsx files >>>> > minify javascript >>>> > >>>> > In order to remove Grunt from the application, we will need some >>>> other tool >>>> > to execute these listed tasks. >>>> > >>>> > There are other tools available, like Gulp.js, that we could use >>>> instead of >>>> > Grunt. We would like to understand your motivation for removing Grun= t >>>> so we >>>> > can find a better solution. >>>> >>>> The motivation is simple - we want a solution that works for the whole >>>> app, can handle debug vs. release execution, pluggable modules, and >>>> installations in read-only directories. >>>> >>>> Per the other thread on the subject (that Joao suggested continuing >>>> discussion on), Surinder is currently looking into flask-webpack. I >>>> spent some time playing with grunt and some other options last week. >>>> >>>> However; this patch is supposed to be about the history tab rewrite. >>>> Whatever solution we use for webpacking/transpiling/linting/minifying >>>> etc, it should be a standalone change as it's decidedly non-trivial. >>>> >>>> -- >>>> Dave Page >>>> Blog: http://pgsnake.blogspot.com >>>> Twitter: @pgsnake >>>> >>>> EnterpriseDB UK: http://www.enterprisedb.com >>>> The Enterprise PostgreSQL Company >>>> >>> >>> >> > --94eb2c0d9ece97292005512fcd04 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi<= /div>

Review comments:

1) First patch contains changes r= elated to copy row feature which is already in process in another thread. A= ny reason to add those changes in this patch as this patch is meant to cont= ain infrastructure changes ?

2) Can the changes related to FeatureTest or Jasmine test be separated = out in another patch?

Thanks,
=
Surinder


On Thu, Jun 1, 2017 at 1:59 AM, M= atthew Kleiman <mkleiman@pivotal.io> wrote:
Hey Surinder,

Is it possible to run th= e task listed above by configuring them in "scripts" in package.j= son file and running concurrently ?

That would be cleaner than what we proposed. We could use the yarn run feature to do just what you are suggesting. It works the same as in npm.=
In package.json -=C2=A0<= /div>
"scripts": {
=C2= =A0 "linter": "eslint pgadmin/static/jsx/**/*.jsx pgadmin/st= atic/js/selection/*.js regression/javascript/**/*.jsx regression/javas= cript/**/*.js *.js"
}

To call it on the command line = -
yarn run linter
=
- Matt

On Wed, M= ay 31, 2017 at 12:35 AM, Surinder Kumar <surinder.kumar@ente= rprisedb.com> wrote:
<= div dir=3D"ltr">
On Wed, May 31, 2017 at 12:01 AM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
=
The motivation is simple - we want a solution that works for the whole=
app, can handle debug vs. relea= se execution, pluggable modules, and
installations in read-only directories.
With the current configuration of Grunt, all the requirements you mention = are available.
The tasks on Grunt should only be run under de= velopment or before we are creating the installer.
=


The installer should pick up only the bundled Javascript = and should install it in the correct folders, this way there is no need to = rewrite or process files in read-only directories of the end user machine.<= /div>

Per the other thread on the subject (that Joao suggested continuing=
discussion on), Surinder is cur= rently looking into flask-webpack. I
spent some time playing with grunt and some other options last week.
We should continue the discussion about flask= -webpack or Grunt or any other Build system in the other thread, but we nee= d to have some decision because this patch needs a build system.
= If we remove Grunt from this patch we will need to execute the following co= mmand every time the application run:

yarn run eslint pgadmin/static/jsx/**/*.jsx pgadm= in/static/js/selection/*.js regression/javascript/**/*.jsx regression/= javascript/**/*.js *.js=C2=A0&& yarn run webpack -- --config webpac= k.config.js && python web/pgAdmin4

And t= o run the jasmine tests:

yarn run eslint pgadmin/static/jsx/**/*.jsx pgadmin/static/js/selection= /*.js regression/javascript/**/*.jsx regression/javascript/**/*.js *.j= s=C2=A0&&=C2=A0yarn run = karma start

And to run the featu= re tests:

yarn r= un eslint pgadmin/static/jsx/**/*.jsx pgadmin/static/js/selection/*.js= regression/javascript/**/*.jsx regression/javascript/**/*.js *.js=C2=A0&am= p;& yarn run webpack=C2=A0-- --config=C2=A0webpa= ck.config.js && python runtests.py
Is this a solution that is acceptable?=C2=A0
<= /blockquote>

As per = my knowledge, Grunt can run the tasks and bundle the JS and CSS files (usin= g r.js which is not preferred). But Webpack can not do both. It can only bu= ndle files, it has to be used with either Grunt or Gulp to execute tasks.

Is = it possible to run the task listed above by configuring them in "scrip= ts" in package.json file and running concurrently ?

scripts: {
=C2= =A0"task-name1": "yarn run eslint filename",
=E2=80=8B = =E2=80=8B
"task-name2": "yarn run karma start", so= on...
}

=
and run them using following command:
npm run task-name1
=C2=A0
<= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;pa= dding-left:1ex">

However; this patch is supposed to be a= bout the history tab rewrite.
Wh= atever solution we use for webpacking/transpiling/linting/minifying
etc, it should be a standalone change as it's decidedly non-trivi= al.
We split this patch into 2 different com= mits:
1 - Add React and all the tools needed to work with it, thi= s includes Grunt, Webpack and ESLint
2 - Change the history tab

Do you want a single commit for each of the followi= ng?=C2=A0
=C2=A0 React (just adds the React library via yarn)
=C2=A0 Webpack (adds Webpack library via yarn and creates the webpac= k.config.js)
=C2=A0 ESLint (adds ESLint library via yarn, creates= .eslintrc config file, and corrects all lint errors that previously existe= d)
=C2=A0 Grunt (adds Grunt library via yarn and creates Gruntfil= e.js config, creating a pipeline of the previous three libraries/tasks)
=C2=A0 Our change to History

Thanks
Joao & Matt=C2=A0

On Tue, May 30, 2017 at = 10:10 AM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, May 30, 2017 at 2:47 PM, Matthew Kleiman <mkleiman@pivotal.io> wrote:
> Hi Dave,
>
> We are currently using the Grunt taskrunner to run the fo= llowing tasks:
>
> lint the javascript code
> start javascript tests
> invoke webpack to transpile and bundle js and jsx files
> minify javascript
>
> In order to remove Grunt from the application, we will need some other= tool
> to execute these listed tasks.
>
> There are other tools available, like Gulp.js, that we could use inste= ad of
> Grunt. We would like to understand your motivation for removing Grunt = so we
> can find a better solution.

The motivation is simple - we want a solution that works for the who= le
app, can handle debug vs. release execution, pluggable modules, and
installations in read-only directories.

Per the other thread on the subject (that Joao suggested continuing
discussion on), Surinder is currently looking into flask-webpack. I
spent some time playing with grunt and some other options last week.

However; this patch is supposed to be about the history tab rewrite.
Whatever solution we use for webpacking/transpiling/linting/minifying<= br> etc, it should be a standalone change as it's decidedly non-trivial.
<= br> --
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company




--94eb2c0d9ece97292005512fcd04--