[Pachi] GTP and other patches
pasky at ucw.cz
Tue Aug 7 16:24:27 CEST 2012
On Tue, Jul 31, 2012 at 10:23:24PM +0100, Neil Mclean wrote:
> I've created a new fork with the branches 'gtp' and 'gcc_win32_cleanup'.
> I've split up the gtp commit that 'moved files around and also changed files'.
> Smaller commits is the way to go. I'm surprised how nice git is to
> use, wow, very powerful!
Great. :) However, the state that I see in the gtp branch still seems
to mix code changes and movement? Perhaps you forgot to push out the
last state of your branch?
Couple of comments about the gcc_win32_cleanup branch:
* 93b3a1193055e209b7236f2691a9b320c847eebf and
7abba6201bc83897c0e569f51542b71588f33aaf have some followup commit
amends. Do you think you could squash them together?
* 7abba6201bc83897c0e569f51542b71588f33aaf: I'm not sure if pattern.h
is the best place for such a random Win32 prototype. It would fit better
* 93b3a1193055e209b7236f2691a9b320c847eebf: I just don't see the logic
change in this commit + next one?
* 91b6a019e11f628473ea44f39336551b771545ae: 'static inline' is more
usual wording than 'inline static' - purely just a nit, feel free to
* I was ambivalent about all the unused parameters changes, but now
I think it's probably a fine cleanup. Nice work! Any reason to have
it split over commits just like it is now? To me, it would make sense to
split this to three commits, one adding the unused tag, another removing
unused parameters, and the final one adding -Wextra.
* 3336931c63f281e1b0a7081fb57776d40e87f23b: The tag cannot be named
__unused as underscore-starting identifiers are a reserved namespace.
Perhaps just UNUSED? It seems you have some extra empty lines after the
definition in util.h.
> Have you tried parameter tuning using Remi's CLOPS? I've heard good
> things about it.
Yes, we definitely tried CLOP, and I think we have incorporated some
changes inspired by the results. It is a nice tool.
Petr "Pasky" Baudis
Smart data structures and dumb code works a lot better
than the other way around. -- Eric S. Raymond
More information about the Pachi