Clean up quip table conversion code

RESOLVED FIXED in Bugzilla 2.18

Status

()

RESOLVED FIXED
17 years ago
6 years ago

People

(Reporter: bbaetz, Assigned: bbaetz)

Tracking

({polish})

2.17
Bugzilla 2.18
x86
Linux
polish

Details

Attachments

(1 attachment, 1 obsolete attachment)

v2
2.35 KB, patch
bugreport
: review+
bugreport
: review+
Details | Diff | Splinter Review
(Assignee)

Description

17 years ago
The badly-indented error messages are annoying. What we should do is convert
data/comments iff its there and there aren't any comments in the db.

We especially don't want to tell people that we created this backup file and
they need to get rid of it - if we can tell them that we could remove it
ourselves...
> The badly-indented error messages are annoying. 

What's wrong with that indentation style?

> What we should do is convert
> data/comments iff its there and there aren't any comments in the db.

No; some admins may have other ways of adding to data/comments.

If data comments exists, we should append values to the DB (even if it already
contains some comments), and rename the file. I wanted to just delete it, but
the original patch author objected and I didn't feel strongly enough to argue.
If you feel it should be deleted, let's just delete it.

Gerv

Comment 3

17 years ago
The indenting style, I take no credit for, as I was happy to continue changing
it to whatever it's supposed to be.

The reason I preferred just renaming (but leaving the file) was to not cause
data loss if someone's upgrading to a temporary database first - my upgrade
process for b.g.o has been load a clean copy of the db, run
checksetup/test/develop, then dump the db and reload. If we were silently nuking
the data/comments file, I'd be pretty annoyed if I don't have a backup. This
gets even worse if you're appending the comments... If you want it changed, rock
on, but that's my reasoning as to why it's a bad idea.
(Assignee)

Comment 4

17 years ago
I didn't mean indenting in the source code, but rather in the output:

The data/comments.bak file can be removed, as it's no longer
           used.

doesn't look very nice.

Its also annoying - it either can be removed, in which case bz should do it, or
its a harmless backup file, in which case we shouldn't touch it.

> No; some admins may have other ways of adding to data/comments.

I don't follow that - it doesn't matter how stuff was in data/comments; if its
there, then we convert it. Or do you mean stuff added after the db was
converted? I don't think we want to care about that.
OK, I get the indent stuff now :-)

Re: the backup file. Fine. Let's move it to .bak, mention it in the message,
then ignore it for evermore. I'm fine with that.

Gerv
just a kink to throw in the loop here...

Got two landfill installs...  one was created by cloning the other.  Both use
the same database because neither is testing any schema changes.

Since one was created by cloning the other one, they both had the same quips. 
Since the quips got appended, now we have duplicate quips.

Not sure if we care, but thought I'd mention it.
Also, a fresh install runs the conversion code - data/comments is checked in
with a 0 size. Firstly, we shouldn't be running the code on a 0-sized file, and
secondly, we should CVS-remove it.

Gerv
(Assignee)

Comment 8

16 years ago
data/comments isn't checked in; checksetup.pl creates it when it creates teh
data dir - my patch removes that along with the other fu.
(Assignee)

Comment 9

16 years ago
Created attachment 95683 [details] [diff] [review]
v2

OK, try this, instead
Attachment #93286 - Attachment is obsolete: true
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Keywords: patch, polish, review
Target Milestone: --- → Bugzilla 2.18

Comment 10

16 years ago
Comment on attachment 95683 [details] [diff] [review]
v2

r=, 2xr by joel
Attachment #95683 - Flags: review+
(Assignee)

Comment 11

16 years ago
Checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 16 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.