Re: [PATCH]Reduce messages in setup.log
On Mon, Jan 28, 2002 at 08:00:36PM -0800, Michael A Chase wrote: >A 2 io_stream_file.cc-patch [applica/octet-stre, quoted, 0.4K] >A 3 archive.cc-patch [applica/octet-stre, quoted, 1.5K] >A 4 compress.cc-patch [applica/octet-stre, quoted, 1.3K] >A 5 compress_bz.cc-patch [applica/octet-stre, quoted, 1.3K] >A 6 compress_gz.cc-patch [applica/octet-stre, quoted, 1.4K] >A 7 io_stream.cc-patch[applica/octet-stre, quoted, 0.7K] >A 8 io_stream_cygfile.cc-patch[applica/octet-stre, quoted, 0.5K] I don't know how Robert prefers this, but it is customary to provide a single patch file not a bunch of separate attachments. With one patch file you can just say patch < foo rather than patch < foo1 patch < foo2 patch < foo3 etc. cgf
Re: [PATCH]Reduce messages in setup.log
=== - Original Message - From: "Christopher Faylor" <[EMAIL PROTECTED]> To: <[EMAIL PROTECTED]> Sent: Tuesday, January 29, 2002 3:14 PM Subject: Re: [PATCH]Reduce messages in setup.log > On Mon, Jan 28, 2002 at 08:00:36PM -0800, Michael A Chase wrote: > >A 2 io_stream_file.cc-patch [applica/octet-stre, quoted, 0.4K] > >A 3 archive.cc-patch [applica/octet-stre, quoted, 1.5K] > >A 4 compress.cc-patch [applica/octet-stre, quoted, 1.3K] > >A 5 compress_bz.cc-patch [applica/octet-stre, quoted, 1.3K] > >A 6 compress_gz.cc-patch [applica/octet-stre, quoted, 1.4K] > >A 7 io_stream.cc-patch[applica/octet-stre, quoted, 0.7K] > >A 8 io_stream_cygfile.cc-patch[applica/octet-stre, quoted, 0.5K] > > I don't know how Robert prefers this, but it is customary to provide a > single patch file not a bunch of separate attachments. With one patch > file you can just say Yes please, one patch is nicer. Rob
Re: [PATCH]Reduce messages in setup.log
- Original Message - From: "Robert Collins" <[EMAIL PROTECTED]> To: <[EMAIL PROTECTED]> Sent: Monday, January 28, 2002 20:25 Subject: Re: [PATCH]Reduce messages in setup.log > - Original Message - > From: "Christopher Faylor" <[EMAIL PROTECTED]> > To: <[EMAIL PROTECTED]> > Sent: Tuesday, January 29, 2002 3:14 PM > Subject: Re: [PATCH]Reduce messages in setup.log > > > > On Mon, Jan 28, 2002 at 08:00:36PM -0800, Michael A Chase wrote: > > > > I don't know how Robert prefers this, but it is customary to provide a > > single patch file not a bunch of separate attachments. With one patch > > file you can just say > > Yes please, one patch is nicer. Sorry. I got confused and thought it was the other way around. What about the compress_gz.error() and compress_bz.error() messages. The gz one is commented out and the bz one isn't. Should they be the same? If so, which is preferred? I lean toward writing both as long as they are going to setup.log.full. -- Mac :}) ** I normally forward private questions to the appropriate mail list. ** Ask Smarter: http://www.tuxedo.org/~esr/faqs/smart-questions.htm Give a hobbit a fish and he eats fish for a day. Give a hobbit a ring and he eats fish for an age.
Re: [PATCH]Reduce messages in setup.log
=== - Original Message - From: "Michael A Chase" <[EMAIL PROTECTED]> To: <[EMAIL PROTECTED]> Sent: Tuesday, January 29, 2002 3:41 PM Subject: Re: [PATCH]Reduce messages in setup.log > - Original Message - > From: "Robert Collins" <[EMAIL PROTECTED]> > To: <[EMAIL PROTECTED]> > Sent: Monday, January 28, 2002 20:25 > Subject: Re: [PATCH]Reduce messages in setup.log > > > - Original Message - > > From: "Christopher Faylor" <[EMAIL PROTECTED]> > > To: <[EMAIL PROTECTED]> > > Sent: Tuesday, January 29, 2002 3:14 PM > > Subject: Re: [PATCH]Reduce messages in setup.log > > > > > > > On Mon, Jan 28, 2002 at 08:00:36PM -0800, Michael A Chase wrote: > > > > > > I don't know how Robert prefers this, but it is customary to provide a > > > single patch file not a bunch of separate attachments. With one patch > > > file you can just say > > > > Yes please, one patch is nicer. > > Sorry. I got confused and thought it was the other way around. No probs. If you have mulitple patchs for _different_things- newlib + cinstall, or w32api + cinstall, then, yes, multiple files are needed. > What about the compress_gz.error() and compress_bz.error() messages. The gz > one is commented out and the bz one isn't. Should they be the same? If so, > which is preferred? I lean toward writing both as long as they are going to > setup.log.full. I think you've missed the point of the messages. They indicate incomplete functions, so that the main log shows what the *program* should have detected. compress_gz::error returns an internal state error value. compress_bz::error returns 0! Likewise for everything that logs to setup.log - it should stay there IF and only IF it's not properly implemented. I will happily accept patches addressing the core issues, but not to hide them :}. I thought when you initially described this that there where a bunch of messages that *shouldn't* be going to the log, but until I review the body of your patches, I can't meaningfully confirm on a per function basis. Rob
Re: [PATCH]Reduce messages in setup.log
- Original Message - From: "Robert Collins" <[EMAIL PROTECTED]> To: "Michael A Chase" <[EMAIL PROTECTED]>; <[EMAIL PROTECTED]> Sent: Monday, January 28, 2002 20:55 Subject: Re: [PATCH]Reduce messages in setup.log > I think you've missed the point of the messages. They indicate > incomplete functions, so that the main log shows what the *program* > should have detected. > > compress_gz::error returns an internal state error value. > compress_bz::error returns 0! > > Likewise for everything that logs to setup.log - it should stay there IF > and only IF it's not properly implemented. > > I will happily accept patches addressing the core issues, but not to > hide them :}. > > I thought when you initially described this that there where a bunch of > messages that *shouldn't* be going to the log, but until I review the > body of your patches, I can't meaningfully confirm on a per function > basis. I didn't say they shouldn't go to a log, but I didn't think they were useful in setup.log. If the intention of those messages is to highlight methods that should have been overridden but aren't, then setup.log is the right place. -- Mac :}) ** I normally forward private questions to the appropriate mail list. ** Ask Smarter: http://www.tuxedo.org/~esr/faqs/smart-questions.htm Give a hobbit a fish and he eats fish for a day. Give a hobbit a ring and he eats fish for an age.
Re: [PATCH]Reduce messages in setup.log
=== - Original Message - From: "Michael A Chase" <[EMAIL PROTECTED]> > If the intention of those messages is to highlight methods that should have > been overridden but aren't, then setup.log is the right place. Essentially, yes thats why they are there: incomplete methods. When I review your patch I'll be looking at the function status. Rob
Re: [PATCH]Reduce messages in setup.log
- Original Message - From: "Robert Collins" <[EMAIL PROTECTED]> To: "Michael A Chase" <[EMAIL PROTECTED]>; <[EMAIL PROTECTED]> Sent: Monday, January 28, 2002 21:38 Subject: Re: [PATCH]Reduce messages in setup.log > - Original Message - > From: "Michael A Chase" <[EMAIL PROTECTED]> > > > If the intention of those messages is to highlight methods that should > have > > been overridden but aren't, then setup.log is the right place. > > Essentially, yes thats why they are there: incomplete methods. When I > review your patch I'll be looking at the function status. There's no sense spending your time on the patch. I need to change it drastically or withdraw it. If I understand the intent: 1. If a method only returns a constant 0, the message should go to setup.log.full to make it easier to tell where more work may be needed. 2. If a method does more than just return a constant 0, it should not call log(), or if it does, the message is only needed in setup.log. Possible candidates: Performs actual work: compress_bz::peek(), compress_gz::peek(), compress_gz::~compress_gz(), io_stream_cygfile::peek(), and io_stream_file::peek(). Returns NULL: archive::next_file_name(), compress::next_file_name(), and io_stream::factory(). Returns -1: compress_bz::seek() and compress_gz::seek(). -- Mac :}) ** I normally forward private questions to the appropriate mail list. ** Ask Smarter: http://www.tuxedo.org/~esr/faqs/smart-questions.htm Give a hobbit a fish and he eats fish for a day. Give a hobbit a ring and he eats fish for an age.
Re: [PATCH]Reduce messages in setup.log
Ok, I've looked at the patch. Most of the methods are stubs that should warn when used. Some aren't, and I'll try and merge those changes in by hand tonight. Don't bother reissuing the patch - I'm about to cause everyone heartache by removing much of the char * usage (I got sick of memleaks) for a quick-and-dirtyish String class. Sigh, still no STL. Rob
Re: [PATCH]Reduce messages in setup.log
- Original Message - From: "Michael A Chase" <[EMAIL PROTECTED]> > > Essentially, yes thats why they are there: incomplete methods. When I > > review your patch I'll be looking at the function status. > > There's no sense spending your time on the patch. I need to change it > drastically or withdraw it. I appreciate this. I'll take you up on it as well :]. > If I understand the intent: > > 1. If a method only returns a constant 0, the message should go to > setup.log.full to make it easier to tell where more work may be needed. Yes, LOG_TIMESTAMP (so that a casual inspection will have it visible). Likewise for any constant response, with a couple of exceptions: * IF the method will *never* get implemented (ie, do we ever need writeable archives?) Then the message level should be LOG_BABBLE, and it should say something like "Foo::Bar - I wasn't expected to be implemented OR used!" > 2. If a method does more than just return a constant 0, it should not call > log(), or if it does, the message is only needed in setup.log. Yes, LOG_BABBLE for routine non-user-visible importance. Noting that a function is called is usually not needed in fact - thus the // in the compress_gz:: call we discussed earlier. > Possible candidates: > >Performs actual work: compress_bz::peek(), compress_gz::peek(), > compress_gz::~compress_gz(), io_stream_cygfile::peek(), and > io_stream_file::peek(). yah, drop to BABBLE, or just remove the logging completely. >Returns NULL: archive::next_file_name(), compress::next_file_name(), and > io_stream::factory(). factory is important, archive::next_file_name and compress::next_file_name should never get called (ie LOG_TIMESTAMP these) - indicates a non-properly overriden sub class. >Returns -1: compress_bz::seek() and compress_gz::seek(). Right. Well, compress_* and seeking don't mix well IMO, so I'm happy for these to become BABBLERS. Rob
Re: [PATCH]Reduce messages in setup.log
=== - Original Message - From: "Robert Collins" <[EMAIL PROTECTED]> > > 1. If a method only returns a constant 0, the message should go to > > setup.log.full to make it easier to tell where more work may be > needed. > > Yes, LOG_TIMESTAMP (so that a casual inspection will have it visible). > Likewise for any constant response, with a couple of exceptions: > * IF the method will *never* get implemented (ie, do we ever need > writeable archives?) Then the message level should be LOG_BABBLE, and it > should say something like "Foo::Bar - I wasn't expected to be > implemented OR used!" * Or for virtual functions that should be abstract, but can't be (for whatever reason). (ie io_stream::~io_stream()). Rob
RE: [PATCH]Reduce messages in setup.log
> -Original Message- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED]]On Behalf Of Robert Collins > Sent: Tuesday, January 29, 2002 12:05 AM > To: Michael A Chase; [EMAIL PROTECTED] > Subject: Re: [PATCH]Reduce messages in setup.log > > > Ok, I've looked at the patch. > > Most of the methods are stubs that should warn when used. Some aren't, > and I'll try and merge those changes in by hand tonight. Don't bother > reissuing the patch - I'm about to cause everyone heartache by removing > much of the char * usage (I got sick of memleaks) Yeah, I'm cryin' my eyes out here. ;-) > for a > quick-and-dirtyish String class. Sigh, still no STL. Rob, would you care to just graft more functionality onto my embryonic "cistring" class? Or was that the plan? -- Gary R. Van Sickle Brewer. Patriot.
Re: [PATCH]Reduce messages in setup.log
=== - Original Message - From: "Gary R. Van Sickle" <[EMAIL PROTECTED]> > > for a > > quick-and-dirtyish String class. Sigh, still no STL. > > Rob, would you care to just graft more functionality onto my embryonic > "cistring" class? Or was that the plan? I left it to the side for now, but yes I do think they should merge. Rob
Re: [PATCH]Reduce messages in setup.log (Revision 1)
Thanks Michael, I'll apply this - minues the io_stream::~io_stream change (I prefer the current wording because someone could override the destructor, but not set the flag, thus strictly speaking it only _appears_ that they haven't.). Rob === - Original Message - From: "Michael A Chase" <[EMAIL PROTECTED]> To: <[EMAIL PROTECTED]> Sent: Tuesday, January 29, 2002 6:35 PM Subject: [PATCH]Reduce messages in setup.log (Revision 1) > I think this covers the changes we discussed. > -- > Mac :}) > ** I normally forward private questions to the appropriate mail list. ** > Ask Smarter: http://www.tuxedo.org/~esr/faqs/smart-questions.htm > Give a hobbit a fish and he eats fish for a day. > Give a hobbit a ring and he eats fish for an age. > > 2002-01-28 Michael A Chase <[EMAIL PROTECTED]> > > * compress_bz.cc (compress_bz::peek): Remove log() call. > (compress_bz::~compress_bz): Ditto. > (compress_bz::seek): Only write log() message to setup.log.full. > * compress_gz.cc (compress_gz::peek): Remove log() call. > (compress_gz::error): Ditto. > (compress_gz::~compress_gz): Ditto. > (compress_gz::seek): Only write log() message to setup.log.full. > * io_stream_cygfile.cc (io_stream_cygfile::peek): Remove log() call. > * io_stream_file.cc: Remove #include "log.h". > (io_stream_file::peek): Remove log() call. > * io_stream.cc (io_stream::factory): Shortened log() message. > (io_stream::~io_stream): Shortened log() message. > >
