We thought of that one too. That sort of makes it sound like the loader is versioned rather than the template files. Also, it's unclear what it is a loader for without "template".
That said, do you think there is a better place for it to live? We stuck it under utils just because we didn't see anywhere else obvious. On Fri, Jan 27, 2017 at 11:43 AM, Dave Page <dp...@pgadmin.org> wrote: > Good point. How about just versioned_loader? > > On Fri, Jan 27, 2017 at 4:40 PM, George Gelashvili > <ggelashv...@pivotal.io> wrote: > > That would work, but the versioned template loader can load anything > that is > > a template, as it extends from DispatchingJinjaLoader > > > > On Fri, Jan 27, 2017 at 11:39 AM, Dave Page <dp...@pgadmin.org> wrote: > >> > >> sql_loader ? > >> > >> On Fri, Jan 27, 2017 at 4:38 PM, George Gelashvili > >> <ggelashv...@pivotal.io> wrote: > >> > Thanks! > >> > Did you have a name in mind? We're not sure we could come up with a > >> > clear > >> > name in fewer words. > >> > > >> > On Fri, Jan 27, 2017 at 11:27 AM, Dave Page <dp...@pgadmin.org> > wrote: > >> >> > >> >> Scrub that Khushboo - Murtuza, can you review on Monday please? I'm > >> >> being told Khushboo is on the critical path for something else at the > >> >> moment. > >> >> > >> >> On Fri, Jan 27, 2017 at 4:22 PM, Dave Page <dp...@pgadmin.org> > wrote: > >> >> > Very nice indeed! I didn't realise we'd ended up with quite so many > >> >> > duplicated templates. > >> >> > > >> >> > Both patches look good to me - really the only thing that caught my > >> >> > eye was the name versioned_template_loader which is somewhat longer > >> >> > than I'd prefer. > >> >> > > >> >> > As it's a major change, and we're going to be wrapping 1.2 in a > >> >> > little > >> >> > over a week, I'd like some further review before committing. > >> >> > Khushboo, > >> >> > can you take a look first thing on Monday please? If you're happy, > >> >> > I'll commit and ask Fahar to do some testing. > >> >> > > >> >> > Thanks! > >> >> > > >> >> > On Thu, Jan 26, 2017 at 8:48 PM, George Gelashvili > >> >> > <ggelashv...@pivotal.io> wrote: > >> >> >> As a followup, we created this additional patch removing > >> >> >> now-redundant > >> >> >> sql > >> >> >> templates. If you're interested in how we decided what files to > >> >> >> delete, > >> >> >> the > >> >> >> script we used is here. We only removed files that were exactly > >> >> >> identical > >> >> >> between multiple postgres versions for the same feature. > >> >> >> > >> >> >> This patch should be applied on a master patched with > >> >> >> version-aware-sql-template-loader-refactor.diff > >> >> >> > >> >> >> > >> >> >> On Thu, Jan 26, 2017 at 12:22 PM, George Gelashvili > >> >> >> <ggelashv...@pivotal.io> > >> >> >> wrote: > >> >> >>> > >> >> >>> Hi Hackers! > >> >> >>> > >> >> >>> Since we are preparing to add greenplum support, we made a new > >> >> >>> template > >> >> >>> loader to automatically pick the available sql template file that > >> >> >>> corresponds to the postgres version number. > >> >> >>> > >> >> >>> Our next patch will be to remove duplicated sql template files > >> >> >>> since > >> >> >>> many > >> >> >>> of the files are identical between versions. > >> >> >>> > >> >> >>> Cheers, > >> >> >>> George & Tira > >> >> >> > >> >> >> > >> >> >> > >> >> >> > >> >> >> -- > >> >> >> Sent via pgadmin-hackers mailing list > >> >> >> (pgadmin-hackers@postgresql.org) > >> >> >> To make changes to your subscription: > >> >> >> http://www.postgresql.org/mailpref/pgadmin-hackers > >> >> >> > >> >> > > >> >> > > >> >> > > >> >> > -- > >> >> > 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 > >> > > >> > > >> > >> > >> > >> -- > >> 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 >
