On Wed, 2008-08-20 at 23:44 +0200, Sandro Tosi wrote: > > Other sponsors have different requirements, these are mine. > > ;-) > > Fine, I just uploaded -9 revision. I've been trying to test dbishell and the shell syntax is, frankly, completely bizarre and deserves to be MUCH more prominently explained in the documentation. Requiring / as the line end character for compatibility with Oracle? WTF? Then claiming that the char cannot be set to ';' (which most users would expect as the line end char in SQL) again for compatibility with Oracle? I mean, Oracle is still non-free so why penalise free software users for reasons of compatibility with non-free? (It can actually be set to ; the claim is that upstream won't do it but then upstream don't seem active anymore, this upstream release was first uploaded 2002-04-04). It's almost enough to make it worth putting this package into contrib! Completely counter-intuitive. The manpage says this is configurable but does not describe how. The README claims that the EOL variable will configure the char but dbishell does not listen to the EOL variable in the environment. Trying to set EOL as a variable within dbishell does not work and the program insists on using / - it does listen to: prompt> $EOL=; (The '$' prefix is all important.) That should be included in the manpage - first possible opportunity. I'd even suggest that as Oracle is not free, that the Debian package could enforce this behaviour as the default and allow those who want to pay for Oracle to set their own $EOL variable. Or maybe a more complex patch that provides an option to use a more sensible line end char and document that in README.Debian and the changelog. The inability to set this as an environment variable is a real PITA because it prevents the setting of a sane line end char as an alias (hence the idea of a new command line option). That could be an alternative patch - one that forces DBIShell/UTIL.pm to listen to $ENV["EOL"] and only use the old default if nothing is set in the environment. A patch to DBIShell/UTIL.pm should do the trick (with corresponding changes in the manpage and a note in README.Debian as to why this was done. However, if the manpage (and README.Debian) make it clear that the first command new users should probably use is: prompt> $EOL=; then we can avoid confusing those who might already be used to / (ensure the docs reinforce that the $ is essential in that command.) Thankfully, it does *not* have to be terminated by the old line end char, the value of $EOL becomes effective immediately the variable is set with ENTER. (This too should probably be in the docs.) Overall, I'd much prefer a patch that listens to EOL from the environment because then users can use EOL in bash aliases: alias dbishell-mysql='EOL=; dbishell --dsn host=localhost --driver mysql' etc. DSN strings are horrendously complicated and anything that helps users get rid of the need to type the blasted things is good news in my book. This would then be prominently described in the manpage, alongside the description of how to reset the EOL once inside dbishell, as above. Also, there is an Ubuntu bug that you have actually fixed by packaging the README - you can close that bug in the changelog with: (LP: #195316) IIRC, (although you might want to double-check the syntax.) Follow the link from the PTS to see the bug details. It is probably worthwhile trying to fix #495978 in Debian too. There is no good reason to override libs like that. However, there is a different problem - you haven't actually fixed the original bug: #446834 $ grep GENERAL DBIShell/* yes, that catches LICENCE which you have omitted, but did you notice that DBIShell/UTIL.pm includes a verbatim copy of the GPL as well? It is worth removing that in your patch to UTIL.pm. Any reason not to package at least some of the icons? You could add an icon to the debian/menu file, seeing as you use a menu and an icon does exist. The manpage should also state that the driver names are case-sensitive. That is bound to catch people out. I'm not sure what is going on with SQLite - it claims SQLite2 when Debian has SQLite0 and SQLite3. Have you managed to get dbishell to successfully connect to any sqlite database? If SQLite does not work, it should be clearly stated in the docs. If it does, but only for sqlite2 (which no longer exists), then either dbishell needs to support sqlite3 or all sqlite2 support should be removed/disabled and the docs updated to state as much. Hmmm, quite a lot to do. Sorry about that. If you don't fancy doing it, maybe dbishell should be dropped from Debian - orphaned, dead upstream, not a whole lot better than the current clients provided by the database programs themselves (in some cases, quite a bit worse than current clients), doesn't support scripting... The package claims: "It provides all the functionality of a database specific shell such as sqlplus or mysql, but in a database independant manner. In some cases, it will be significantly more advanced than the command line database shell supplied by your database vendor." Sadly, IMHO, that might have been true in 2001 when this was first packaged for Debian - it is very much untrue in 2008, at least as far as I can tell. Usual problem - the others are constantly updated by a large dedicated upstream team, dbishell is showing overt signs of bit rot after SIX years without upstream activity. IMHO it has been superceded (and by a noticeable margin). If you do fancy doing it, maybe you should do this as an ITA and not as a QA upload. It would probably mean trying to take over upstream too (whereupon you can reconcile that horrible Oracle-compatibility stuff). ;-) Overall, I'm not entirely convinced that dbishell is a particularly useful or friendly package - it is rarely better than using whatever client the database provides itself, it has (currently) completely counter-intuitive syntax compared to the database clients and appears to be dead upstream. Are you sure it is worth keeping it in Debian? I'm not convinced. -- Neil Williams ============= http://www.data-freedom.org/ http://www.nosoftwarepatents.com/ http://www.linux.codehelp.co.uk/
Attachment:
signature.asc
Description: This is a digitally signed message part