Re: [patch] Refactor: clipboard, translations, jasmine

From: Atira Odhner <aodhner(at)pivotal(dot)io>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [patch] Refactor: clipboard, translations, jasmine
Date: 2017-03-09 21:56:16
Message-ID: CA+Vc24pTnZCMKAvHja+sPy9MMZCAJ_xo7zV9gwU4nDpTzdKDeQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave,

We've split these changes up into separate patches, added docs, pulled
jasmine out of the packaged app, and enabled running from the commandline:

* Add jasmine*
- it runs with karma from the commandline
- it is outside of the /web folder so that tests and test dependencies
are excluded from the packaged app.

* Enable refactoring javascript with translations:*
- Create a translations.js and translate.js which uses flask to pull in
translations, but
lets us have the flask template rendering confined to that one
file,
enabling us to test javascript files that rely on translations

Refactor copying text to clipboard into a separate file (this
demonstrates use of translate.js)

remove some dead code from sqleditor

Change quotes in sqleditor to make it valid js independent of flask (it
makes our IDE happy)

- The changes to the translation mechanism need discussion. How will
> it work? What do developers have to do differently? When will the
> changes it allows be implemented?

Instead of using flask to render translations into javascript, developers
will need to require "translate" and use it in much the same way as the
flask _ method. So, for example, translate("My name is %(name)s.", {name:
"Tira"}) would replace {{ _("My name is %(name)s.", {name: "Tira"}) }}

We would like to start on these changes right away! We refactored the
clipboard functionality out of sqleditor.js and used this translate
functionality. The resulting file is now testable javascript.

Tira & George

On Mon, Feb 27, 2017 at 6:28 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> On Fri, Feb 24, 2017 at 7:14 PM, Atira Odhner <aodhner(at)pivotal(dot)io> wrote:
> > Hi hackers,
> >
> > We made some changes to start to make the javascript testable:
> >
> > - Move clipboard copying into its own file so we can test the
> > interaction with it when we start adding column selection
> > - Create a translations.js and translate.js which uses flask to pull
> in
> > translations, but
> > lets us have the flask template rendering confined to that one
> file,
> > enabling us to test javascript files that rely on translations
> > - add jasmine javascript testing which currently runs from the
> browser
> > at /static/SpecRunner.html when the app is up.
> > - delete some dead code from sqleditor.js
> >
> > Why we decided to make translate.js only support interpolations of the
> form
> > "%(variable)s":
> > - it matches functionality in python
> > - having named variables allows for multiple interpolations in one
> > sentence without the risk that a translation will swap the order
>
> There are various issues with this patch:
>
> - It needs to be broken up - one change, one patch. There are 4
> distinct changes here, that each need to be reviewed on their own
> merit.
>
> - The changes to the translation mechanism need discussion. How will
> it work? What do developers have to do differently? When will the
> changes it allows be implemented?
>
> - There are no doc updates corresponding to the changes to the
> translation mechanism.
>
> - What does the jasmine testing actually do? There are no doc or
> README updates to describe it.
>
> - There is no update to libraries.txt
>
> - The jasmine code seems to be littered with cruft that we don't want
> to be carrying in our repo. Can it be reduced to the bare minimum
> CSS/JS files?
>
> - Test code (including jasmine) needs to be isolated from the
> application code, e.g. in a tests/ directory. Future versions of
> pgAdmin will exclude the test suite code from their packaging.
>
> > Future things we'd like to add to this:
> > - make jasmine run from the commandline with the rest of the tests
>
> I think that's a pre-requisite for any additional test functionality
> at this point, given that we're moving to fully automated testing.
>
> Thanks, Dave.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Attachment Content-Type Size
0001-Add-jasmine.patch application/octet-stream 8.1 KB
0002-Enable-refactoring-javascript-with-translations.patch application/octet-stream 6.3 KB
0003-Refactor-copying-text-to-clipboard-into-a-separate-f.patch application/octet-stream 8.3 KB
0004-remove-some-dead-code-from-sqleditor.patch application/octet-stream 3.2 KB
0005-Change-quotes-in-sqleditor-to-make-it-valid-js-indep.patch application/octet-stream 23.4 KB
0006-update-libraries.txt.patch application/octet-stream 6.8 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2017-03-10 09:03:32 pgAdmin 4 commit: Update Polish translation
Previous Message Joao Pedro De Almeida Pereira 2017-03-09 19:05:35 Re: [patch] Shows titles for disabled buttons on hover