On Tue, Jun 5, 2018 at 12:28 PM Dave Page wrote:
> Hi
>
> On Mon, Jun 4, 2018 at 10:27 PM, Joao De Almeida Pereira <
> jdealmeidapere...@pivotal.io> wrote:
>
>> Hi Hackers,
>>
>> Attached you can find the patch that introduces electron to our code base.
>>
> Cool. FYI, I'd like to get this into rough shape and then push it to a dev
> branch for fine-tuning. I think it'll be easier to work that way.
>
Sure, you can create a dev branch and push this patch to it.
Great. So here's my initial feedback:
>
> - The Mac build you provided doesn't work for me. It hangs on the loading
> screen.
>
Could we get some more information about the machine? Using the python
from the venv directory, are you able to run the app directly?
cd /Applications/pgAdmin.app/Contents/Resources/app/
venv/bin/python web/pgAdmin4.py
Do you get and error?
We are setting PGADMIN_PORT, PGADMIN_KEY and SERVER_MODE environment
variables prior to starting it.
>
>
- A number of the changes are related to the naming of requirejs. I'd be
> inclined to pull that out into a separate patch and get it committed to
> master ASAP.
>
This change only makes sense in the Electron context. Neverthless fell free
to add it to master if you think it is relevant.
> - I think the build instructions need to be more generic (particularly on
> macOS). For example, I do not use HomeBrew (largely due to some nasty
> security issues they had in the past). I was able to mostly port the
> instructions and build script over to work using MacPorts (without PyEnv)
> which actually turned out to be somewhat more simple than what's there now.
>
Since we don't use MacPorts, we cannot provide installation instructions.
> - I'm not sure what this is intended to do: "git checkout electron".
> Clearly that isn't correct.
>
That was the name of our development branch. It can be removed.
>
> - All new builds should be using Python 3.6. We need to deprecate 2.7 as
> there are some Unicode related issues that cannot be fixed in it.
>
For Windows, we are using 2.7 because of external library compilation
issues. Let us know if you are able to get around this or how to make
this work.
> - I would like to see the new build code adapted to follow the existing
> conventions as much as reasonable, to avoid having to change build systems
> or other processes/procedures that folks use. For example, build scripts
> should be under pkg/, completed packages left in dist/, build staging done
> in xxx-build directories rather than elsewhere.
>
That sounds reasonable.
> - It may be a result of my use of MacPorts, but I'm getting the following
> failure building:
>
> yarn run v1.3.2
> $ electron-forge make --platfrom=darwin --arch=x64 --targets=dmg
> ✔ Checking your system
> ✔ Resolving Forge Config
> We need to package your application before we can make it
> ✔ Preparing to Package Application for arch: x64
> ⠼ Compiling ApplicationFailed to compile file:
> /var/folders/c6/pwf0k2d509s2xx6vh0h633vmgn/T/electron-packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/Contents/Resources/app/venv/lib/python3.6/site-packages/setuptools/command/launcher
> manifest.xml
> Compiling
> /var/folders/c6/pwf0k2d509s2xx6vh0h633vmgn/T/electron-packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/Contents/Resources/app/venv/lib/python3.6/site-packages/setuptools/command/launcher
> manifest.xml resulted in a MIME type of application/xml, which we don't
> know how to handle
> ⠋ Compiling ApplicationFailed to compile file:
> /var/folders/c6/pwf0k2d509s2xx6vh0h633vmgn/T/electron-packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/Contents/Resources/app/web/pgadmin/browser/server_groups/servers/templates/servers/supported_servers.js
> /var/folders/c6/pwf0k2d509s2xx6vh0h633vmgn/T/electron-packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/Contents/Resources/app/web/pgadmin/browser/server_groups/servers/templates/servers/supported_servers.js:
> Unexpected token (6:7)
> ⠦ Compiling ApplicationFailed to compile file:
> /var/folders/c6/pwf0k2d509s2xx6vh0h633vmgn/T/electron-packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/Contents/Resources/app/web/pgadmin/browser/templates/browser/js/endpoints.js
> /var/folders/c6/pwf0k2d509s2xx6vh0h633vmgn/T/electron-packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/Contents/Resources/app/web/pgadmin/browser/templates/browser/js/endpoints.js:
> Unexpected token (5:6)
> Failed to compile file:
> /var/folders/c6/pwf0k2d509s2xx6vh0h633vmgn/T/electron-packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/Contents/Resources/app/web/pgadmin/browser/templates/browser/js/messages.js
> /var/folders/c6/pwf0k2d509s2xx6vh0h633vmgn/T/electron-packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/Contents/Resources/app/web/pgadmin/browser/templates/browser/js/messages.js:
> Unexpected token (37:1)
> Failed to compile file:
> /var/folders/c6/pwf0k2d509s2xx6vh0h633vmgn/T/electron-packager/darwin-x64/pgAdmin-darwin-x64/Electron.app/Contents/Resources
