Skip site navigation (1) Skip section navigation (2)

Re: New patch for bugs in GPDB csv format log handling

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Chuck McDevitt <cmcdevitt(at)greenplum(dot)com>
Cc: "pgadmin-hackers(at)postgresql(dot)org" <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: New patch for bugs in GPDB csv format log handling
Date: 2009-06-08 21:05:25
Message-ID: 937d27e10906081405r77c86dafp5137ab72d2b9f389@mail.gmail.com (view raw or flat)
Thread:
Lists: pgadmin-hackers
On Mon, Jun 8, 2009 at 10:02 PM, Chuck McDevitt<cmcdevitt(at)greenplum(dot)com> wrote:
> Thanks Dave.  Some questions:
>
>> Hi,
>>
>> Some issues with this, 2 minor, one less so:
>>
>> - Please rearrange the code in frmStatus.h to remove the class
>> definition. We try to keep the headers purely for declarations and
>> only include one-line functions in them. I would suggest a new file in
>> utils/ given that it's a generally useful class.
>
> I would have done this already, but was worried you wouldn't accept new files this late in the cycle.
> I will put the class definition in its own file.   Should I also move the class declaration to its own .h file?
> I think that would be cleaner.

New files are fine. I'd rather that, than cram new general purpose
classes into the header for a window class.

>> - Status bar messages such as _("Reading log from server.") should
>> generally end with ... to indicate the task is ongoing.
>
> Will do.
>
>>
>> - Messages passed to wxLogError should be translatable, and explain
>> the problem to the user without being phrased as a question (wxT("Log
>> line does not start with timestamp?: %s \n")). Messages passed to
>> wxLogNotice needn't be translated however.
>
> Will do.  Hopefully, this can only be hit if a log file is corrupted.
> Or should this just be a wxLogNotice, since the code is supposed to just recover any continue without any user interaction.
> I'd made it a LogError mostly to help with my debugging when I was working on the code.

I'm fine with wxLogNotice.


-- 
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com

In response to

pgadmin-hackers by date

Next:From: Guillaume LelargeDate: 2009-06-08 21:23:53
Subject: Re: pgAgent "new step" fails for batch jobs in pgAdmin3 1.10.0 Beta 2
Previous:From: Chuck McDevittDate: 2009-06-08 21:02:32
Subject: Re: New patch for bugs in GPDB csv format log handling

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group