274266 - Server type should not be hardcoded in SQL values

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, defect)

Description

[Vlad Dascalu]

We plan to land PG-SQL, Sybase, etc support. The type of the SQL server should
not be hard-coded in SQL configuration variables from checksetup.pl

Comment 1

[Vlad Dascalu]

Attachment: v1

Comment 2

[Dave Miller [:justdave]]

This probably isn't worth landing until we actually do support another database.

Comment 3

[Vlad Dascalu]

Why take the baby-steps approach? :-)

Instead of waiting one big patch to land this, we can split it up and add
progressive support for that. Smaller patches are easier to review and we'll
reach there in shorter time.

Comment 4

[Dave Miller [:justdave]]

This isn't a baby step.  It's fixing something that's not broken (yet).

Comment 5

[GavinS]

Vladd, I am trying to review this quickly - honest - but I'm looking at all the
other places where mysql is mentioned in checksetup.pl, and considering the
earlier comments from justdave as well...

My first review request looked so easy at first....

I'll get it done tomorrow.

Comment 6

[GavinS]

Comment on attachment 168533 [details] [diff] [review]
v1

The changes in the patch are fine, and nice and simple, but I'm going to r-
(sorry) because they are not really consistent within themselves (is it 'SQL
database' or 'SQL server', or perhaps it should just be 'database'?). There are
also some other places in checksetup.pl where 'MySQL' could be replaced to try
and tidy it up which it would be nice to do at the same time -- if this was
going to happpen.

If you *really* wanted, then you could probably persuade me to r+ because this
is a simple comment change which is not going to break anything, but from
justdave's comments and dependency inddication, I doubt that you would get
approval for this change anytime soon anyway??

(This could maybe be duped to bug#248175)

Comment 7

[Frédéric Buclin]

Attachment: patch, v2

PostgreSQL is now supported. Most of the things reported in the previous patch
have already been fixed.

Comment 8

[Myk Melez [:myk] [@mykmelez]]

a=myk, although I would think this should be "database server".

Comment 9

[Frédéric Buclin]

Attachment: patch submitted

replace 'SQL server' by 'database server'. This is the patch I commited.

Comment 10

[Frédéric Buclin]

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.404; previous revision: 1.403
done