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 08:48:01
Message-ID: CA+OCxowW_6u52Kujv7hAhPuUnUpZRFKrQ8bA8X-VtFERHmTC6g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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?

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)c
>>>>> om> 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 08:52:20 pgAdmin 4 commit: Fix an issue where base types are not displaying in t
Previous Message Dave Page 2017-09-13 08:47:04 pgAdmin 4 commit: Run webpack in production mode when building for rele