[Nitro] [PATCH] glue-validations-to-og
bryan.a.soto at gmail.com
Fri Apr 14 19:04:04 EDT 2006
On 4/13/06, Dimitri Aivaliotis <aglarond at gmail.com> wrote:
> > Patch is available here if you're interested. I'd be quite happy with
> > more testers. :) I don't think it will make it in the repo till after
> > 0.30 though if then.
> I did see the patch. I was hesitant about applying because of the STI
> issues. I haven't yet converted my legacy apps over to Og, so I don't
> know what effects the patches will have on a real application.
STI issues? Was it you that mentioned Postgres provides table
inheritance? Og follows it as quasi-outlined at:
all the subclasses are stored in a single table. Of course, that is
based off of databases not supporting actual table inheritance. :)
The only other STI issue is that postgres was determining whether a
class is STI differently from the other stores. The patch addresses it
by removing the method overrides the postgres adapter does. It
actually removes a lot of postgres code and reuses from the
superclass. Has the pleasant benefit of making the stores function
similarly as well as being the textbook beginnings of refactoring. :)
> > Thanks for your feedback on the patch and the test suite. It could
> > definitely use some improvements. :)
> Well, here's some more. :)
> With the patches (glue-validations-to-og and og-psql-refactoring) applied:
> tc_override doesn't work because it assumes that SQLite3 is installed.
Never noticed now that I have them all installed. :/
> tc_reverse doesn't work because the KirbyBase store doesn't support
> the 'exec' method.
Kirby doesn't do alot of things Og needs unfortunately. :(
Another thing I need to do is use conditional code depending on which
store we are using. I think the sql that is currently exec'd is mysql
> tc_inheritance, tc_inheritance2, and store/tc_sti still all give:
> nitrohq/og/lib/og/store/psql.rb:635:in `exec': ERROR: column "ogtype"
> duplicated (PGError)
Hmm... I think that has to do with Dylan's Kirby patch where he made
ogtype an Og property. I liked the idea and added a ticket to do that
for the other stores. Thanks for reminding me on that one...
> 1) Failure:
> test_all(TC_OgAggrCalc) [./test/og/tc_aggregations_calculations.rb:41]:
> <28> expected but was
This one is the test. The other databases sort the result set.
Postgres doesn't. The consensus on the list was to not assume the
results were sorted. I need to do that...
> 2) Failure:
> test_all(TestMultiple) [./test/og/tc_multiple.rb:37]:
> <false> is not true.
Bad test as well. Needs to be modified and removed.
> Am I running something incorrectly here?
Not at all. You're just finding things I've missed or forgotten.
Forgetfullness is the reason I've opened so many tickets.
By the way, have I mentioned how glad I am for people that test
patches? (Hint, hint ;)
Many thanks to you and Kashia for taking the time to review and
comment on them. :)
"Never tell people how to do things. Tell them what to do and they
will surprise you with their ingenuity." —General George S. Patton
More information about the Nitro-general