Server type should not be hardcoded in SQL values

RESOLVED FIXED in Bugzilla 2.20

Status

()

Bugzilla
Installation & Upgrading
RESOLVED FIXED
13 years ago
5 years ago

People

(Reporter: Vlad Dascalu, Assigned: Frédéric Buclin)

Tracking

2.19.2
Bugzilla 2.20
Bug Flags:
approval +

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

13 years ago
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
(Reporter)

Comment 1

13 years ago
Created attachment 168533 [details] [diff] [review]
v1
Assignee: zach → vladd
Status: NEW → ASSIGNED
(Reporter)

Updated

13 years ago
Attachment #168533 - Flags: review?(bugzilla)
This probably isn't worth landing until we actually do support another database.
Depends on: 98304
(Reporter)

Comment 3

13 years ago
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.
This isn't a baby step.  It's fixing something that's not broken (yet).

Comment 5

13 years ago
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

13 years ago
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)
Attachment #168533 - Flags: review?(bugzilla) → review-
(Reporter)

Updated

13 years ago
Assignee: vladd → nobody
Status: ASSIGNED → NEW
(Assignee)

Comment 7

13 years ago
Created attachment 186011 [details] [diff] [review]
patch, v2

PostgreSQL is now supported. Most of the things reported in the previous patch
have already been fixed.
Assignee: nobody → LpSolit
Attachment #168533 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #186011 - Flags: review?(mkanat)
(Assignee)

Updated

13 years ago
Target Milestone: --- → Bugzilla 2.20

Updated

13 years ago
Attachment #186011 - Flags: review?(mkanat) → review+

Updated

13 years ago
Flags: approval?
Version: unspecified → 2.19.2
a=myk, although I would think this should be "database server".
Flags: approval? → approval+
(Assignee)

Comment 9

13 years ago
Created attachment 186255 [details] [diff] [review]
patch submitted

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

Comment 10

13 years ago
Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.404; previous revision: 1.403
done
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.