On Mon, Jun 09, 2003 at 06:55:50PM +0200, Michael Nordstrom wrote: > Hmm, I don't think I want hard-coded paths in the code base when > they can be avoided. This line is from armlets/armletldscript: > > SEARCH_DIR(/usr/arm-palmos/lib); > > IMO, the armletldscript could be "created" by the configure script > after it has figured out the SEARCH_DIR setting.
Actually, this argument seems moot in my environment where my SEARCH_DIR would have been /usr/local/palm/arm-palmos/lib/ .. is this really required? > Other things I saw in the patch, > > - #ifndef __ENDIANUTILS_H__ > > No names with leading and trailing underscores. This endianutils.h file was actually included as an example from palm for building armlets. The code itself is straight forward enough and free for general use. Maybe just the defines in there should be moved into an armlets.h file and made to conform plucker's standards in there. > - "DEFINES = -DNEWGCC" doesn't make much sense in the Makefile > > It is included in the viewer's Makefile because we can't run > 'make depend" without it (the ZLib library requires the option) > > I saw that you have also included it in the cleaner app's > Makefile; it could (and probably should) be removed. This I never was too sure about. However, I will remove it off of the cleaner app as well as any future patches for this. > - the viewer.rcp file doesn't depend directly on the source code > in the armlets dir; it depends on the armlets.rcp file. Actually, it technically does. If the source to an armlet changes, it needs to be re-built, which then spits out a new .bin file. That binary file is included into viewer.rcp via armletdata.rcp. If it changes then then pilrc needs to be re-ran so that the new .bin is included in the prc. > - the $(FONTDIR)/*.txt dependency should be removed, not the > $(FONTDIR)/*.rcp dependency, since the dependency is on the > rcp file. Same reason as above. If the source to the font changes, pilrc needs to be re-ran to include the new binaries. > - coding guidelines! I didn't think it was that bad, except for a few variable names could be more specific. Can you provid esome examples? > - the config option should be --enable-armlets, i.e. the default > is off. Boooo. Armlets are generally a good thing! And should be included if we can build it. Besides, if arm-palmos-gcc and its cronies aren't found its skipped anyways. Although the overhead in this is only ~500 bytes, it would be nice to keep the default as on unless it's impossible to build. You know, innocent until proven guilty? :) -- Adam McDaniel Array.org Calgary, AB, Canada _______________________________________________ plucker-dev mailing list [EMAIL PROTECTED] http://lists.rubberchicken.org/mailman/listinfo/plucker-dev
