Hi On Mon, Nov 11, 2019 at 7:01 AM Aditya Toshniwal < aditya.toshni...@enterprisedb.com> wrote:
> Hi Dave, > > On Thu, Nov 7, 2019 at 7:56 PM Dave Page <dp...@pgadmin.org> wrote: > >> >> >> On Thu, Nov 7, 2019 at 2:18 PM Dave Page <dp...@pgadmin.org> wrote: >> >>> >>> On Thu, Nov 7, 2019 at 1:25 PM Akshay Joshi < >>> akshay.jo...@enterprisedb.com> wrote: >>> >>>> Thanks, patch applied. >>>> >>>> On Thu, Nov 7, 2019 at 6:39 PM Aditya Toshniwal < >>>> aditya.toshni...@enterprisedb.com> wrote: >>>> >>>>> Hi Hackers, >>>>> >>>>> Attached is the updated patch with few more changes and corrections. >>>>> Kindly review. >>>>> >>>> >>> I've committed a couple of minor tweaks - one to remove a space, e.g. >>> >>> gettext('A page refresh is required to apply the theme. Do you wish to >>> refresh the page now ?'), >>> >>> is now: >>> >>> gettext('A page refresh is required to apply the theme. Do you wish to >>> refresh the page now?'), >>> >>> And another change to fix the word wrapping in the README which was >>> totally different from the rest of the file. >>> >>> Thanks! >>> >> >> Oh, and do we need all the code in config.py? I really didn't even want a >> config option in there to turn theming on or off (what's the point?), let >> alone 20 new lines. >> > The code is added after the config_local and config_distro is loaded. So, > user won't be able to disable it unless he directly changes the config.py. > That is clearly wrong and needs to be fixed. config_local and config_distro should be able to override anything in config.py. But... why allow the themes to be updated or disabled at all? It's not like a non-developer can add new ones, and it's not a security issue that an administrator might need to control. In fact, it's arguably an accessibility feature, for those whose eyes (like mine) last the day better with a darker theme. Let's remove it entirely please. I don't see any good reason to have any of that in config.py. Thanks. > I'll reduce the code a bit. > >> >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company >> > > > -- > Thanks and Regards, > Aditya Toshniwal > Sr. Software Engineer | EnterpriseDB India | Pune > "Don't Complain about Heat, Plant a TREE" > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company