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-12 10:57:12
Message-ID: CAM5-9D87YoUJjTFfWek5jSq3Mjs4HU+XDK1RgWvZg0KHTOatfQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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
>

Attachment Content-Type Size
bundle_script_changes.patch application/octet-stream 1.7 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Willy-Bas Loos 2017-09-12 11:32:31 Re: v2.0-rc1 not starting, runtime not building
Previous Message Dave Page 2017-09-12 10:45:30 Re: v2.0-rc1 not starting, runtime not building