David Dreezer > > That sounds good, but I'd still have a few comments which I believe could even > improve the code: > > - I think find $cfdir -type f -iname *.asc (or maybe even just -name) would be > safer;
Fair point. I chose i to be more friendly, but isn't something I'm firm on. > well, wouldn't even ls $cfdir/package_config/*.asc do the trick? To be > honest, I don't know what the best place for keeping those files would be. > Hmm, no, your code seems to assume that those files are in $cfdir directly, > otherwise the sed command would break it!? I'm slightly confused. > This is a patch to make-fai-nfsroot, thus the $cfdir is doesn't necessarily include /package_config/ I'm pretty sure. I am reasonably sure that I'd want to keep the key file someplace that makes sense for the make-fai-nfsroot and my sense was that because /package_config/ doesn't necessarily even have to exist for make-fai-nfsroot to run successfully it wouldn't have been the best place. I thought that /etc/fai/ or even better, etc/fai/apt would be more appropriate and that $cfdir was the expression of that within the script. > - Maybe the most important change would therefore be a comment about the > location of those files *and* documentation in the man page. > I'll be sure to add a comment. I don't know how to go about submitting a change to a man page. Is it just a patch too? This is pretty new to me, yet. > - Why copy the file? AFAIK there is no need to store it, > "cat $cfdir/$f | $ROOTCMD apt-key add -" should do as well. I chose to copy the file so that some time in the future, should the question arise, I'll be able to answer the inevitable "how did that key get there?" I'm an old guy, I tend to forget things and I code in such a way as to leave myself hints. I'd be perfectly happy to not do that, and use your recommendation, however. > > Hope this helps, > Michael >
