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

From: Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
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:06:16
Message-ID: CAM5-9D-vPCfcr9HFf2TUm-Uty-+FzfP=EDYDdBCwJpYeo9020A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers


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`​

​ ​
},
});

> 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
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2017-09-13 09:22:58 Re: [pgAdmin4][Patch]: Run MakeFile to generate builds in production mode
Previous Message Dave Page 2017-09-13 08:59:11 pgAdmin 4 commit: Update server and database icons with a clearer desig