Discussion:
[Modeling-users] patch for quoted entities, attributes
John Lenton
2004-07-07 08:37:01 UTC
Permalink
Attached is a patch that quotes entites and attributes in the sql. It
has several problems, still:

- I haven't fixed the tests (and I'm not certain it's even possible
without rewriting them), so if you use postgres suddenly a whole
slew of tests fail.

- Schema generation *almost* works: I'm not quoting the drop
statements for the primary key indexes.

- I changed an unrelated bit: I set useAllCaps to default to 0 in
Entity. This makes the previous bug hide under the carpet, plus
you don't have to quote things when using 'psql' (the latter is the
reson for this change).

- No documentation.

- Only done for postgres (but the per-adapter changes are small)

- Some of the lines are way too long, especially when cutting a line
would mean creating an intermediate object---in those cases I
defer to Sébastien to decide if it's worth it---or when the original
was pretty long to start with :)

however, it's working for me (so far!), and I'm using it for
development.
--
John Lenton (***@gmail.com) -- Random fortune:
bash: fortune: command not found
John Lenton
2004-09-03 01:30:10 UTC
Permalink
Sébastien,

I know this might be imposing too much, but could you possibly tell me
what I could to this patch to make it accepted into Modeling?
--
John Lenton (***@gmail.com) -- Random fortune:
bash: fortune: command not found
Sebastien Bigaret
2004-09-03 11:28:03 UTC
Permalink
Hi John,
Post by John Lenton
Sébastien,
I know this might be imposing too much, but could you possibly tell me
what I could to this patch to make it accepted into Modeling?
Sorry, this slipped out out my mind; and worse, the post itself was left
unanswered as far as I can see in the archives. Okay, so, first: I have
no problem with the feature itself. Second, the problems you list (the
tests not being updated yet, etc.) should definitely be addressed
before it's integrated.

And now for a slightly OT discussion: I'm sorry for any inconvenience
that the delay in answering the request has caused. In fact and as you
probably noticed, I've really had problems addressing issues in a
reasonable time those last months, there's a lot of pending bugs,
patches waiting for approval or further discussion, etc. Not to speak
about the 0.9 release candidate waiting, obvisoulsy. Well, this is
all done on my spare time, which is not that big those days... What I
can say is that I'm currently considering reorganizing things so that
at least pending requests do not wait too long --more to come in the
coming weeks.

Back on your specific request, what would accelerate the integration in
the framework would be, definitely, to address the issues listed in your
original post (tests, making this work for the other adaptors).

On the feature itself, I think I'd prefer not to make it the default,
but controlled by setting the appropriate variable, and probably
adding an env. variable for triggering it. What do you think? Can you
also comment on your change on Entioty.externalNameForInternalName()
where the default for useAllCaps is changed from '1' to '0' ?


Oh, and last: it would help a lot (at least, for me not to forget about
it!) if you could add an entry for this in the patch section.


-- Sébastien.

Loading...