opencog-dev team mailing list archive

[Gustavo Machado Campagnani Gama <gama@xxxxxxxxxxxxx>]

Hi Trent,

On Fri, 2008-06-06 at 14:50 +1000, Trent Waddington wrote:
> Let's try this again shall we? :)
> 
>   http://bazaar.launchpad.net/~qg/opencog/dev/revision/779
> 
> I reverted all changes on my tree, merged from main and then made my port.
> 
> I ignored all warnings (about 65), in order to keep the size of the patch down.

I've merged the changes from rev 779 (and 784) of your branch to the
current trunk. I've applied a few corrections here and there so I'd
appreciate if you could checkout and test the result of the merge (it
builds and passes all tests on linux, but I don't have a win32
environment yet).

It's available at lp:~gama/opencog/dev.

Once we have an agreement about this, I'll cherry-pick the remaining
relevant revisions from your branch.
	
> Oh, and if you accept 779, please take 780 also.  It's just the build
> files, committed separately to aid your review.

Isn't cmake supposed to generate the .vcproj project files? It seem to
me that adding these files to the repository would be the same as adding
the Makefile's (as opposed to CMakeLists.txt, or Makefile.am for
autotool-based projects).

> Same with 781.  The getopt implementation for win32.

You chose to create a separate "win32" dir in this patch. So far, all
the platform specific changes were done using #ifdefs and adapters (such
as numeric.h or hash_map.h). Shouldn't we use the same approach here? Or
perhaps a different file name? (I've seem projects using getopt_win32.h.
But as I've said previously, I'm not exactly experienced with the best
practices to write win32-friendly C++ code so I'd like to listen to
other opinions).

I saw that you have added quite a few other changes to your branch. I
have a few quick comments regarding them:

  - 786: Fix bad try/catch style. All control paths of a non-void 
    function should return a value.

    I'd like to know what's wrong with this try/catch style, since I'm 
    probably guilty of this as well.

  - 793: Merge 773 back in.  This fixes the command prompt and adds the 
    'close' command. 

    This one is no longer required. It's been merged already.

  - 794: Fix vcproj for CogServer.. it was using the same intermediate 
    dir as libCogServer which means a pdb was getting clobbered and 
    MSVC was having a cry. 

    This one depends on rev. 780 and I'll probably merged them if we
    accept 780.

Thanks,

--
Gustavo