<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta http-equiv="Content-Type" content="text/xhtml; charset=utf-8">
</head>
<body>
<div style="font-family:sans-serif"><div style="white-space:normal"><p dir="auto">Any thoughts on the PR ?</p>
<br><p dir="auto">On 26 Jul 2017, at 20:55, Francisco Obispo wrote:</p>
</div>
<blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px"><div id="801B7B19-A9D4-4C00-B6D1-1B7E96DACAD8">
<div style="font-family:sans-serif"><div style="white-space:normal">
<p dir="auto">I’ve created a pull request for this feature:</p>
<p dir="auto"><a href="https://github.com/dbsrgits/dbix-class-schema-loader/pull/13" style="color:#3983C4">https://github.com/dbsrgits/dbix-class-schema-loader/pull/13</a></p>
<p dir="auto">It works by adding: <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">-o omit_view_definitions=1</code> to the loader options.</p>
<p dir="auto">I’ve tested it in my computer, and seems to work correctly, </p>
<p dir="auto">best regards,</p>
<p dir="auto">On 25 Jul 2017, at 12:24, Matt S Trout wrote:</p>
<p dir="auto"></p></div>
<div style="white-space:normal"><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px"><p dir="auto">On Fri, Jul 21, 2017 at 03:14:33PM -0700, Francisco Obispo wrote:</p>
<blockquote style="border-left:2px solid #777; color:#999; margin:0 0 5px; padding-left:5px; border-left-color:#999"><p dir="auto">Yes, but wasn’t sure were to post the request.</p>
</blockquote><p dir="auto">The Schema-Loader queue would've been ok, here is probably best anyway.<br>
</p>
<blockquote style="border-left:2px solid #777; color:#999; margin:0 0 5px; padding-left:5px; border-left-color:#999"><blockquote style="border-left:2px solid #777; color:#BBB; margin:0 0 5px; padding-left:5px; border-left-color:#BBB"><blockquote style="border-left:2px solid #777; color:#BBB; margin:0 0 5px; padding-left:5px; border-left-color:#BBB"><p dir="auto">The result is that `.pm`’s derived from a view from `dbicdump` are<br>
larger than they need to be, inherently using more memory, and with<br>
no real good purpose. There should be an option to turn this<br>
behavior off.</p>
</blockquote><p dir="auto">While I don't see any reason we wouldn't take a patch to supply<br>
such an<br>
option, I confess to being surprised that this was noticeable -<br>
how did<br>
you measure the memory usage with and without, and how big was the<br>
difference?<br>
</p>
</blockquote><p dir="auto">The diffs in git started getting very big all of the sudden, and in<br>
the company that I work, that started raising questions on the QA<br>
team, plus we have some areas where we have fairly large postgres<br>
views whose complexity is completely abstracted in a simple<br>
structure that has no interest to the ORM.</p>
</blockquote><p dir="auto">Personally, I quite like being able to see when the meaning of one of<br>
my result classes just changed, but I can absolutely see "this is<br>
beyond our abstraction boundary and therefore we don't want it in the<br>
diff" as an argument.<br>
</p>
<blockquote style="border-left:2px solid #777; color:#999; margin:0 0 5px; padding-left:5px; border-left-color:#999"><p dir="auto">Those fairly large views, force us to store the definition in RAM<br>
(as a perl string) now just for the sake of documentation, but in<br>
reality, at least in our system, is the wrong place to store the<br>
view definition. I don’t want to have to explain a more junior<br>
developer that the real view lives in SQL, and what lives in the .PM<br>
is just a snapshot of how it looked when the schema was dumped.</p>
</blockquote><p dir="auto">Talking about RAM usage - which I suspect is negligible - is an incredibly<br>
weak argument here and if you don't have numbers I'd suggest just not making<br>
that argument.<br>
<br>
"This is outside our abstraction boundary" is a good argument.<br>
<br>
"This is making the diffs when we update the schema ugly" is a good argument.<br>
<br>
"This might confuse junior developers" is a good argument.<br>
<br>
All of those three are IMO each sufficient to justify a patch.<br>
<br>
So, what I'd suggest is, that you look at putting together a patch that<br>
<br>
(a) allows to toggle whether the SQL should be in code, docs, or nowhere<br>
(b) emits a comment immediately above where view_definition normally is<br>
that mentions it's reflected from the schema and mentions the option<br>
<br>
That way junior developers using existing systems shouldn't be confused<br>
anymore because the comment will tell them, and the existence of the comment<br>
should also mean anybody with a use case like yours will find the option.<br>
<br>
Plus it can also emit a comment when it *doesn't* reflect the view<br>
information, so if somebody later wants to switch to DBIC-controlled<br>
deployment they'll also find the option to tweak.<br>
<br>
I don't think the default should change, though, because I know of quite a<br>
few projects that rely on Schema::Loader'ed schemas being deployable to set<br>
up test databases and similar, and if anybody with needs like yours can easily<br>
find the option anyway, it seems impolite to break their setups.<br>
<br>
Does that all make sense? Fancy having a go at a patch?<br>
<br>
-- <br>
Matt S Trout - Shadowcat Systems - Perl consulting with a commit bit and a clue<br>
<br>
<a href="http://shadowcat.co.uk/blog/matt-s-trout/" style="color:#777">http://shadowcat.co.uk/blog/matt-s-trout/</a> <a href="http://twitter.com/shadowcat_mst/" style="color:#777">http://twitter.com/shadowcat_mst/</a><br>
<br>
Email me now on mst (at) shadowcat.co.uk and let's chat about how our CPAN<br>
commercial support, training and consultancy packages could help your team.<br>
<br>
_______________________________________________<br>
List: <a href="http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class" style="color:#777">http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class</a><br>
IRC: irc.perl.org#dbix-class<br>
SVN: <a href="http://dev.catalyst.perl.org/repos/bast/DBIx-Class/" style="color:#777">http://dev.catalyst.perl.org/repos/bast/DBIx-Class/</a><br>
Searchable Archive: <a href="http://www.grokbase.com/group/dbix-class@lists.scsys.co.uk" style="color:#777">http://www.grokbase.com/group/dbix-class@lists.scsys.co.uk</a></p>
</blockquote></div>
<div style="white-space:normal">
</div>
</div></div></blockquote>
<div style="white-space:normal"><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px">
</blockquote><p dir="auto">_______________________________________________<br>
List: <a href="http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class" style="color:#3983C4">http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class</a><br>
IRC: irc.perl.org#dbix-class<br>
SVN: <a href="http://dev.catalyst.perl.org/repos/bast/DBIx-Class/" style="color:#3983C4">http://dev.catalyst.perl.org/repos/bast/DBIx-Class/</a><br>
Searchable Archive: <a href="http://www.grokbase.com/group/dbix-class@lists.scsys.co.uk" style="color:#3983C4">http://www.grokbase.com/group/dbix-class@lists.scsys.co.uk</a></p>
</div>
</div>
</body>
</html>