Re: [pgAdmin4][Patch]: Run MakeFile to generate builds in production mode

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: Run MakeFile to generate builds in production mode
Date: 2017-09-13 09:22:58
Message-ID: CA+OCxoy7U3FrPWyKkeGoFqEmXCxMSB6aeXgSQOKpX4CY3JRuxw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Wed, Sep 13, 2017 at 10:06 AM, Surinder Kumar <
surinder(dot)kumar(at)enterprisedb(dot)com> wrote:

>
> ​
> On Wed, Sep 13, 2017 at 2:18 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Thanks, committed - though I did add a bundle-dev target to the Makefile
>> for convenience.
>>
>> I'm a little concerned though that the query history still doesn't work
>> in dev mode. Is that fixable?
>>
> ​That's an issue with ReactJS and I had logged a ticket​
>
> ​on their repository but didn't get any response from them.
> The solution is pass `production` flag in place of `envType` in `dev` mode
> too.
>
> const definePlugin = new webpack.DefinePlugin({
> ​ ​
> 'process.env': {
> ​ ​
> 'NODE_ENV': JSON.stringify(envType),
> ​ // In dev mode envType is `development`​
>
> ​ ​
> },
> });
>

What would be the effect of that? No source map being generated for React?

> ​
>
>
>
>> On Tue, Sep 12, 2017 at 11:57 AM, Surinder Kumar <
>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Dave,
>>>
>>> I had discussed this with Ashesh and according to him, running `yarn run
>>> bundle` should always build prod bundle. So, I made changes accordingly.
>>>
>>> Following changes are made in package.json script commands:
>>>
>>> 1) Running `yarn run bundle` will build prod bundle.
>>>
>>> 2) Running `yarn run bundle:dev` will build dev bundle.
>>>
>>> No changes are required in Makefile and build.sh files.
>>>
>>> Please review and let me know for changes.
>>>
>>> Thanks,
>>> Surinder
>>>
>>> On Tue, Sep 12, 2017 at 2:39 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>> Hi
>>>>
>>>> This is over-complicating things. As I said, we never build packages in
>>>> dev mode - there's just no need.
>>>>
>>>> All we need is easy-to-use Makefile targets that allow us to do either
>>>> a prod or a dev bundle. The packages should always use prod bundles.
>>>>
>>>> I assume with the (to-be-updated) version of this patch,
>>>> webpack_config_changes.patch is still required?
>>>>
>>>> On Tue, Sep 12, 2017 at 6:28 AM, Surinder Kumar <
>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> As per the review comment given by Ashesh:
>>>>>
>>>>> 1) The test cases with target `check:` must run in `prod` mode by
>>>>> default.
>>>>>
>>>>> 2) Add another target `check-dev:` to run test cases in `dev` mode.
>>>>>
>>>>> So, the patch is updated with fixed review comments.
>>>>>
>>>>> Please find an updated patch and review.
>>>>>
>>>>> Thanks,
>>>>> Surinder
>>>>>
>>>>>
>>>>> On Tue, Sep 12, 2017 at 10:28 AM, Surinder Kumar <
>>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Dave,
>>>>>>
>>>>>> I have added a new target to run build in 'dev' mode. By default, it
>>>>>> will run in production mode.
>>>>>>
>>>>>> By dev mode it means, run `yarn run bundle:dev` and by prod mode, run
>>>>>> `yarn run bundle:prod`.
>>>>>>
>>>>>> In Mac bundle, added new target `make appbundle-dev` to generate
>>>>>> build with command `yarn run bundle:dev`. The default target `make
>>>>>> appbundle` will run `yarn run bundle:prod`.
>>>>>>
>>>>>> In Windows bundle, If Make.bat file is executed with an argument
>>>>>> `dev` like `./Make.bat x86 dev`, it will call `yarn run bundle:dev`
>>>>>> otherwise it will run `yarn run bundle:prod`
>>>>>>
>>>>>> Please find attached patch and review.
>>>>>>
>>>>>> Thanks,
>>>>>> Surinder
>>>>>>
>>>>>> On Fri, Sep 8, 2017 at 12:28 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>
>>>>>>> No, let's just have two targets in one makefile. It's only for our
>>>>>>> convenience.
>>>>>>>
>>>>>>> --
>>>>>>> Dave Page
>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>> Twitter: @pgsnake
>>>>>>>
>>>>>>> EnterpriseDB UK:http://www.enterprisedb.com
>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>
>>>>>>> On 8 Sep 2017, at 05:34, Surinder Kumar <
>>>>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> On Thu, Sep 7, 2017 at 8:48 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> On Thu, Sep 7, 2017 at 7:28 AM, Surinder Kumar <
>>>>>>>> surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> When we run Webpack in production mode, it performs optimization
>>>>>>>>> on code while in development we don't optimize generated JS and CSS bundles
>>>>>>>>> as dev mode is for developer use.
>>>>>>>>>
>>>>>>>>> So we should run Webpack bundle in production mode when we are
>>>>>>>>> generating bundles for release mode.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Don't we also need a bundle target for dev mode? The patch changes
>>>>>>>> "make bundle" to run "yarn run bundle:prod"
>>>>>>>>
>>>>>>> ​yes I think so.
>>>>>>> We can have two Makefiles - one for dev mode and other for
>>>>>>> production mode.
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> *In the second patch:*
>>>>>>>>>
>>>>>>>>> 1) Enabled "*sourced maps*" in production mode as well which will
>>>>>>>>> help in debugging issues.
>>>>>>>>>
>>>>>>>>> 2) Removed "*yarn run linter*" script when Webpack runs in
>>>>>>>>> production mode because it is for developer only to check if there are any
>>>>>>>>> syntax errors in JS modules.
>>>>>>>>>
>>>>>>>>> 3) Removed redundant script command "*yarn run bundle*" as "*yarn
>>>>>>>>> run bundle:dev*" does the same thing.
>>>>>>>>>
>>>>>>>>> Please find an attached patch.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Surinder
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> 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
>>>>
>>>
>>>
>>
>>
>> --
>> 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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Surinder Kumar 2017-09-13 09:34:55 Re: [pgAdmin4][Patch]: Run MakeFile to generate builds in production mode
Previous Message Surinder Kumar 2017-09-13 09:06:16 Re: [pgAdmin4][Patch]: Run MakeFile to generate builds in production mode