public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ashesh Vashi <[email protected]>
To: Anthony Emengo <[email protected]>
Cc: Joao De Almeida Pereira <[email protected]>
Cc: Akshay Joshi <[email protected]>
Cc: Dave Page <[email protected]>
Cc: Murtuza Zabuawala <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Cc: Khushboo Vashi <[email protected]>
Subject: Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree
Date: Fri, 11 May 2018 12:01:53 +0530
Message-ID: <CAG7mmowkNtkoLGQFeWAH-TSOi+QWbHT2-RbMmjRDAMAxMufmeg@mail.gmail.com> (raw)
In-Reply-To: <CAG8BBZOKQDKtyu1-2HVCP5HFy1rKFdPVrMTY0SPEhb8x7Q7PBA@mail.gmail.com>
References: <CAE+jja=Gdd032H7tpoZD2C0m2R7SnTZpHX_oPx2K2zGbaaW9yg@mail.gmail.com>
<CAFOhELcrZsRkwV4beOC7UT_HYvi9Rk=HFJ355g8Bj4bqyzMLog@mail.gmail.com>
<CAE+jjaneGOR_R7QvYnZt=khumRsyhnrzFRhpMG5-Q+rR9guLzQ@mail.gmail.com>
<CAKKotZROcoO_rtrrJUTvpfgSh1w4ig1ogQrkDrKSQHuKEL7CdQ@mail.gmail.com>
<CAE+jjakEZtn0ct128Xd+es5xCZ5eZLwoNZw6X0ue7oFB+a83oQ@mail.gmail.com>
<CAFOhELe_t_QGBbcYYm3Zrfwf3z9674PY5sy2QS46wHDTf-8DHA@mail.gmail.com>
<CAE+jja=u+W+FNaWGVs1KPZm1eYA_F=7ktomSbmsxOo-cjuOdBw@mail.gmail.com>
<CAE+jja=TZzMXOq4+t=dA+LfTGkW66urB37Tt9sEJ_UjAiymbbQ@mail.gmail.com>
<CAE+jjamSM_pBLNmYLn6-hWMiQvToceV2iPuME-eH5c+tgaYf7A@mail.gmail.com>
<CA+OCxowN15jg_DV4o+R+OSczyGHHccnJ1sPXE_XukYugaTKyyw@mail.gmail.com>
<CA+OCxozTMcEDKZEu9YCOsSBSed23Ka8hHiO5s6VF76KopbipNQ@mail.gmail.com>
<CAG7mmowrM3b8cTY+zcCAcVma8CKWHwXSF3=BUDcSFLpjH=jQBg@mail.gmail.com>
<CAE+jjanjQ295x0DgffT8OARRv8fge5KOPJpMCnLWe+j0jdqGJQ@mail.gmail.com>
<CAG7mmoym9838PVKdDD=469Bsx6AFBV1ThMK_dyitUmG26147Qg@mail.gmail.com>
<CAG7mmoy=5W3scnPs-TVpcu49F7gZU2qySPaakwfEdcg6rg2RMQ@mail.gmail.com>
<CAG8BBZOAFgw6iDseWTSEUC+tiwc0zCoQyycXGFWbW6ueftCggA@mail.gmail.com>
<CAG7mmoyirqdDGZnXcz5odMBnsOYY83Dj6Uw8gV59OPTJSnaMAg@mail.gmail.com>
<CA+OCxoz3C+HRDzh05SKhMjSZPnHezf=F0eiNoo_HHqUtiJWPiQ@mail.gmail.com>
<CAG7mmoxjfZGA8Gp4HMByTAf6ZLZ7Oaj2yTUrgbOrpJ0H7OtQNQ@mail.gmail.com>
<CAE+jja=GFVqodrWJ+vzBYO4tzoZipKqdThKUqMOGyoZPtJc1+Q@mail.gmail.com>
<CAG7mmozE+8pneLOp8RokHZiav0kcM2Ts6NkHwrz2n31-Ykn_kA@mail.gmail.com>
<CA+OCxowNhQ7waRSPp5P--dFisLFkf+EEOwH+y0+t+ZT9dXLXTA@mail.gmail.com>
<CAE+jjamkYzDxjvSmdkU-1Gyys1FXtrXGr+d7=yrsAREmXsteOQ@mail.gmail.com>
<CAE+jja=vaQOE6XbJH34+N24CZY1DRdYz8wgkxxTx5KSaqw7pLQ@mail.gmail.com>
<CAE+jjakrO-RaK=m5YYUBrF=9SgYjOyU59bZo+vWxDoFXG75Jvg@mail.gmail.com>
<CAE+jjanc7WbL6k42=Dp8NBw9mdiv4jccK7t5JfN2rD2n8NiYTg@mail.gmail.com>
<CAFOhELfyS404C50JWdgV16sBO7WYAJYYOD80kBA1AEhREALCmw@mail.gmail.com>
<CANxoLDfyOFt2QDAXE5BVC6X8tHZV4q3K=_gRiuWN_ife9FYU_w@mail.gmail.com>
<CAE+jjamBMWfWCypF_h5x0NThhmGHx7to6HsJ66WSmJrL6UdQ+g@mail.gmail.com>
<CAG7mmoyjWCTZUXH-CBTpgNy4ErA_6ojRgd+dT63zdAkGMY_2JQ@mail.gmail.com>
<CAG8BBZOKQDKtyu1-2HVCP5HFy1rKFdPVrMTY0SPEhb8x7Q7PBA@mail.gmail.com>
On Thu, May 10, 2018 at 8:08 PM, Anthony Emengo <[email protected]> wrote:
> 1. In TreeNode, we're keepging the reference of DOMElement, do we really
>> need it?
>
> As of right now, our Tree abstraction acts as an adapter to the aciTree
> library. The aciTree library needs the domElement for most of its functions
> (setInode, unload, etc). Thus this is the easiest way to introduce our
> abstraction and keep the functionality as before - at least until we decide
> that whether we want to switch out the library or not.
>
I understand that. But - I've not seen any reference of domElement the code
yet, hence - pointed that out.
> 2. Are you expecting the tree class to be a singleton class
>
> Since this tree is referenced throughout the codebase, considering it to
> be a singleton seems like the most appropriate pattern for this usecase. It
> is very much the same way how we create a single instance of the aciTree
> library and use that throughout the codebase. Moreover, it opens up
> opportunities to improve performance, for example caching lockups of nodes.
> I’m not a fan of singletons myself, but I feel like we’re simply keeping
> the architecture the same in the instance.
>
Yeah - I don't see any usage of tree object from anywhere.
And, we're already creating new object in browser.js (and, not utitlizing
that instance anywhere.)
-- Thanks, Ashesh
>
>
> Sincerely,
>
> Anthony and Victoria
>
view thread (69+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree
In-Reply-To: <CAG7mmowkNtkoLGQFeWAH-TSOi+QWbHT2-RbMmjRDAMAxMufmeg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox