[Gtkradiant] CVS: reviewed code again, updated todo

Fri, 16 May 2003 05:25:31 -0500

This is a mixed plain/HTML MIME encoded message.

--105308073114813
Content-Type: text/plain

User   : timo
Branch : bug800-spog_branch
Root   : zerowing:/cvs
Date   : 2003/05/16  05:25:01

reviewed code again, updated todo

--

  GtkRadiant/docs/developer/CHANGES
    1.377.2.2 : http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/docs/developer/CHANGES?rev=1.377.2.2
    diff :
      that we distribute with the binaries. (see changelog)

    ! 12/05/2003
    ! SPoG
    ! - Fixed win32 build error (win32 has no <dirent.h>).
    ! - Added dir_good() check to handle failure of dir_open() in posix DIR wrapper.
    ! - Renamed dynamic_string_t and path_t to DynamicString and UnixPath respectively.
    ! - Removed unnecessary use of 'inline' and 'virtual' keywords in class definitions (bad habit).
    ! - Documented archive interface.
    ! 
    ! 09/05/2003
      TTimo
    ! - some easy fixes to get it to startup on Linux
    ! - tagging the current source as bug800-spog, preparing rollback
    ! - reviewed the code for all the broken stuff:

    ! A list of broken things with bug800 patches:

    ! http://www.qeradiant.com/wikifaq/index.php?Code%20Conventions
    ! name classes correctly, name headers correctly, put them in the right places

    ! class path_t

    ! - ibytestream.h: remove this file completely. use IDataStream and extend idatastream.h header

    ! - vfspk3/archive.cpp: typedef DIR* dir_t; <- bloat, using DIR* is fine

    ! - WARNING: directory path does not end with separator
    ! happens a lot during startup
    ! GtkRadiant convention is to use trailing / on all directory paths

    ! - class DirectoryListVisitor : public IArchive::IVisitor
    ! don't declare classes inside classes. useless
    ! specially in this case

    ! - IVisitor to it's own header

    ! - all those inline keywords are bloat. don't expect some inline keywords in the middle of STL bloat to make a perf difference

    ! - class DirectoryListVisitor : public IArchive::IVisitor
    ! declared and implemented at the same time in vfspk3/archive.cpp
    ! use declaration, then implementation

    ! - class DirectoryListVisitor : public IArchive::IVisitor
    ! virtual statements in all those are useless. there are no children to override. only needed on the upper level class method

    ! - DirectoryArchive::release: trash, use refcount class

      05/05/2003
      SPoG
    --- 1,72 ----
      This is the changelog for developers, != changelog for the end user 
      that we distribute with the binaries. (see changelog)

    ! 16/05/2003
      TTimo
    ! - http://zerowing.idsoftware.com/bugzilla/show_bug.cgi?id=815
    !   found out about string_t that slept through my reviews
    ! 
    ! Updated list of broken stuff on this branch:

    ! - include/bytestream.h: remove this file, extend idatastream.h header
    ! from the code conventions:
    !   include/ directory:
    !   This directory is holding the API header files for the synapse modules (plus a few build control and version related headers)
    !   All headers that describe synapse APIs should start with an i: ishader.h igl.h 

    !   The few files in include/ that don't start with a lowercase i are specific files for build control and configuration.
    !   Regular developement never creates non i-prefixed files in include/
    !   
    ! the merge with IDataStream goes like this (note the corrected class names):
    ! class IInputStream
    ! class IOutputStream

    ! class IDataStream : public IInputStream, IOutputStream

    + class ISeekableStream
    + class ISeekableInputStream : public IInputStream, public ISeekableStream
    + class ISeekableOutputStream : public IOutputStream, public ISeekableStream

    ! typedef unsigned int size_type; goes away, use size_t

    ! - include/iarchive.h

    ! renamed VisitorFunc to IArchiveVisitor
    ! move it out of IArchive class (keep it in iarchive.h though)
    ! from code conventions:
    !   Don't declare classes inside classes. Makes the code harder to read and isn't useful to anything.

    ! - remove libs/bytestreamutils.h
    ! put that functionality into the IStream stuff / idatastream.h stuff
    ! anything you retrieve from an IStream should be endian-correct

    ! - libs/filestream.h
    ! I'd rather see a header without embedded code, and a .cpp file for it

    ! - libs/fs_filesystem.h libs/fs_path.h
    ! those totally lack documentation about what they are doing
    ! same as above. way too much embedded code. makes things harder to maintain. implement in a .cpp
    ! and as usual, name the classes correctly, C and I prefixes

    ! - gamespecific_t: rename correctly to CGameSpecific
    ! I like the idea of gathering all hardocded game-specific behaviours inside one same class.
    ! But is it really the topic of bug800?

    ! - gamespecific_executable_t: rename correctly to CGameSpecificExecutable
    ! shouldn't this be merged with CGameSpecific?

    ! 12/05/2003
    ! SPoG
    ! - Fixed win32 build error (win32 has no <dirent.h>).
    ! - Added dir_good() check to handle failure of dir_open() in posix DIR wrapper.
    ! - Renamed dynamic_string_t and path_t to DynamicString and UnixPath respectively.
    ! - Removed unnecessary use of 'inline' and 'virtual' keywords in class definitions (bad habit).
    ! - Documented archive interface.
    ! 
    ! 09/05/2003
    ! TTimo
    ! - some easy fixes to get it to startup on Linux
    ! - tagging the current source as bug800-spog, preparing rollback

      05/05/2003
      SPoG

3 files modified :

  GtkRadiant/GtkRadiant.prj
    1.10.2.1 : http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/GtkRadiant.prj?rev=1.10.2.1
    1.10 : http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/GtkRadiant.prj?rev=1.10
    diff : http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/GtkRadiant.prj.diff?r1=1.10&r2=1.10.2.1

  GtkRadiant/docs/developer/CHANGES
    1.377.2.2 : http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/docs/developer/CHANGES?rev=1.377.2.2
    1.377.2.1 : http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/docs/developer/CHANGES?rev=1.377.2.1
    diff : http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/docs/developer/CHANGES.diff?r1=1.377.2.1&r2=1.377.2.2

  GtkRadiant/include/qertypes.h
    1.39.2.2 : http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/include/qertypes.h?rev=1.39.2.2
    1.39.2.1 : http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/include/qertypes.h?rev=1.39.2.1
    diff : http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/include/qertypes.h.diff?r1=1.39.2.1&r2=1.39.2.2

--105308073114813
Content-Type: text/html

<html>
<head><title>CVS report</title></head>
<body bgcolor="#ffffff" text="#000000">

<table border=0 width="100%" cellspacing=0 cellpadding=0>
 <tr>
<td align=left><b>Commit from <i>timo</i> on branch <i>bug800-spog_branch</i></b></td>
  <td align=right valign=bottom>2003/05/16  &nbsp; 05:25:01</td>
 </tr>
</table>
<hr width="100%" size=2 noshade><br>

<code>
reviewed code again, updated todo<br>
</code>
<hr width="100%" size=2 noshade><br>
<code>
<b>GtkRadiant/docs/developer/CHANGES</b>
<pre>  that we distribute with the binaries. (see changelog)

! 12/05/2003
! SPoG
! - Fixed win32 build error (win32 has no <dirent.h>).
! - Added dir_good() check to handle failure of dir_open() in posix DIR wrapper.
! - Renamed dynamic_string_t and path_t to DynamicString and UnixPath respectively.
! - Removed unnecessary use of 'inline' and 'virtual' keywords in class definitions (bad habit).
! - Documented archive interface.
! 
! 09/05/2003
  TTimo
! - some easy fixes to get it to startup on Linux
! - tagging the current source as bug800-spog, preparing rollback
! - reviewed the code for all the broken stuff:

! A list of broken things with bug800 patches:

! http://www.qeradiant.com/wikifaq/index.php?Code%20Conventions
! name classes correctly, name headers correctly, put them in the right places

! class path_t

! - ibytestream.h: remove this file completely. use IDataStream and extend idatastream.h header

! - vfspk3/archive.cpp: typedef DIR* dir_t; <- bloat, using DIR* is fine

! - WARNING: directory path does not end with separator
! happens a lot during startup
! GtkRadiant convention is to use trailing / on all directory paths

! - class DirectoryListVisitor : public IArchive::IVisitor
! don't declare classes inside classes. useless
! specially in this case

! - IVisitor to it's own header

! - all those inline keywords are bloat. don't expect some inline keywords in the middle of STL bloat to make a perf difference

! - class DirectoryListVisitor : public IArchive::IVisitor
! declared and implemented at the same time in vfspk3/archive.cpp
! use declaration, then implementation

! - class DirectoryListVisitor : public IArchive::IVisitor
! virtual statements in all those are useless. there are no children to override. only needed on the upper level class method

! - DirectoryArchive::release: trash, use refcount class

  05/05/2003
  SPoG
--- 1,72 ----
  This is the changelog for developers, != changelog for the end user 
  that we distribute with the binaries. (see changelog)

! 16/05/2003
  TTimo
! - http://zerowing.idsoftware.com/bugzilla/show_bug.cgi?id=815
!   found out about string_t that slept through my reviews
! 
! Updated list of broken stuff on this branch:

! - include/bytestream.h: remove this file, extend idatastream.h header
! from the code conventions:
!   include/ directory:
!   This directory is holding the API header files for the synapse modules (plus a few build control and version related headers)
!   All headers that describe synapse APIs should start with an i: ishader.h igl.h 

!   The few files in include/ that don't start with a lowercase i are specific files for build control and configuration.
!   Regular developement never creates non i-prefixed files in include/
!   
! the merge with IDataStream goes like this (note the corrected class names):
! class IInputStream
! class IOutputStream

! class IDataStream : public IInputStream, IOutputStream

+ class ISeekableStream
+ class ISeekableInputStream : public IInputStream, public ISeekableStream
+ class ISeekableOutputStream : public IOutputStream, public ISeekableStream

! typedef unsigned int size_type; goes away, use size_t

! - include/iarchive.h

! renamed VisitorFunc to IArchiveVisitor
! move it out of IArchive class (keep it in iarchive.h though)
! from code conventions:
!   Don't declare classes inside classes. Makes the code harder to read and isn't useful to anything.

! - remove libs/bytestreamutils.h
! put that functionality into the IStream stuff / idatastream.h stuff
! anything you retrieve from an IStream should be endian-correct

! - libs/filestream.h
! I'd rather see a header without embedded code, and a .cpp file for it

! - libs/fs_filesystem.h libs/fs_path.h
! those totally lack documentation about what they are doing
! same as above. way too much embedded code. makes things harder to maintain. implement in a .cpp
! and as usual, name the classes correctly, C and I prefixes

! - gamespecific_t: rename correctly to CGameSpecific
! I like the idea of gathering all hardocded game-specific behaviours inside one same class.
! But is it really the topic of bug800?

! - gamespecific_executable_t: rename correctly to CGameSpecificExecutable
! shouldn't this be merged with CGameSpecific?

! 12/05/2003
! SPoG
! - Fixed win32 build error (win32 has no <dirent.h>).
! - Added dir_good() check to handle failure of dir_open() in posix DIR wrapper.
! - Renamed dynamic_string_t and path_t to DynamicString and UnixPath respectively.
! - Removed unnecessary use of 'inline' and 'virtual' keywords in class definitions (bad habit).
! - Documented archive interface.
! 
! 09/05/2003
! TTimo
! - some easy fixes to get it to startup on Linux
! - tagging the current source as bug800-spog, preparing rollback

  05/05/2003
  SPoG
</pre>
<table border=0 width="100%">

 <tr>
  <td colspan=5>&nbsp;</td>
 </tr>
 <tr bgcolor="#e0e0e0">
  <td colspan=5 align=center><b>3 files modified</b></td>
 </tr>
 <tr>
  <td><b>Module</b></td>
  <td><b>File name</b></td>
  <td colspan=3><b>Version</b></td>
 </tr>
 <tr>
  <td><b>GtkRadiant</b></td>
  <td><code>GtkRadiant.prj</code></td>
  <td><a href="http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/GtkRadiant.prj?rev=1.10">1.10</a></td>
  <td><a href="http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/GtkRadiant.prj.diff?r1=text&tr1=1.10&r2=text&tr2=1.10.2.1&f=h">&gt;&gt;&gt;</a></td>
  <td><a href="http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/GtkRadiant.prj?rev=1.10.2.1">1.10.2.1</a></td>
 </tr>
 <tr>
  <td><b>GtkRadiant</b></td>
  <td><code>docs/developer/CHANGES</code></td>
  <td><a href="http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/docs/developer/CHANGES?rev=1.377.2.1">1.377.2.1</a></td>
  <td><a href="http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/docs/developer/CHANGES.diff?r1=text&tr1=1.377.2.1&r2=text&tr2=1.377.2.2&f=h">&gt;&gt;&gt;</a></td>
  <td><a href="http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/docs/developer/CHANGES?rev=1.377.2.2">1.377.2.2</a></td>
 </tr>
 <tr>
  <td><b>GtkRadiant</b></td>
  <td><code>include/qertypes.h</code></td>
  <td><a href="http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/include/qertypes.h?rev=1.39.2.1">1.39.2.1</a></td>
  <td><a href="http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/include/qertypes.h.diff?r1=text&tr1=1.39.2.1&r2=text&tr2=1.39.2.2&f=h">&gt;&gt;&gt;</a></td>
  <td><a href="http://zerowing.idsoftware.com/viewcvs/viewcvs.cgi/GtkRadiant/include/qertypes.h?rev=1.39.2.2">1.39.2.2</a></td>
 </tr>

</table>

</body>
</html>

--105308073114813--