davidgiven / ack

The Amsterdam Compiler Kit
http://tack.sf.net
Other
428 stars 62 forks source link

Carl ansi part1 #172

Closed ccodere closed 5 years ago

ccodere commented 5 years ago

Let us try again : This is part 1 of the ANSI conversion, i could not do it in a smaller chunk than this.

Next steps:

Later on:

Next steps should be smaller than this one.

ccodere commented 5 years ago

I will look into it tomorrow, this was building on my machine, but after the merge with the head and latest changes i get on my local machine also:

davidgiven commented 5 years ago

Wow, this was a tonne of work! Thanks very much. I've gon through it and left a few minor comments. LGTM in general.

My biggest remaining concern is that it's too big, and is doing three different things: doing the ansification, adding part of a cmake setup, and replacing some of the source generation scripts.

Could you please move the cmake stuff and the script refactoring into different PRs? The cmake stuff is only partially complete and isn't ready for merge. The new scripts aren't hooked into the build.lua build system, so they're not used anyway, which means they shouldn't be here. We want to ansification stuff to be as simple and mechanical as possible.

But thank you for doing all this! It's really useful.

ccodere commented 5 years ago

Could you please move the cmake stuff and the script refactoring into different PRs? The cmake >stuff is only partially complete and isn't ready for merge. The new scripts aren't hooked into the >build.lua build system, so they're not used anyway, which means they shouldn't be here. We want to >ansification stuff to be as simple and mechanical as possible.

Done, normally CMake stuff and sed scripts are completely removed, not sure if git automatically removes the scripts directory though.

davidgiven commented 5 years ago

Thanks! That's merged --- it looks great. It's a substantially cleaner compile now, too.