I've recently debugged/played with this patch a lot, so I'll throw in a few cents about it [1].
A) a few chunks of code really need to be cleaned up. There are tons of left over debugging that makes the code confusing. But that's easy enough to remove. B) The patch seems to include a library (that I presume) was brought in from a different Dell tool/library/something. This library does sensors, sel, and seemingly a lot of duplicate functionality of other code in ipmitool. Instead of editing the ipmitool code to support the Dell specific stuff, they chose to include their library. I'm not a maintainer of ipmitool, but this feels wrong to include. It might be better to ask the patch writer to modify the original codebase to support their modifications instead of including an entirely new sub-library. (Note, this decision was probably reasonable to save the programmer a lot of time, so I'm not attacking the original patch writer's decision. It's just not what a ipmitool maintainer might want to accept.) C) As far as I can tell, a number of the delloem commands are actually just reformatting of other ipmitool output. Wouldn't we want the delloem patch to only support the new information, not reiterate other information?? If reformatting is important, they could be options to the other commands? D) there is no manpage entry Again, I'm not a maintainer of ipmitool. But these are some issues/things I noticed in the patch. Al [1] - There were bugs in the sensor reading output. I have a good feeling what the fixes would entail, but since I don't maintain that code, I can't really say if it's best/correct or not. On Fri, 2009-07-03 at 11:26 +0200, Tim Bell wrote: > Is there any chance of the patch in > http://*ipmitool.wiki.sourceforge.net/space/filelist making it into the > 1.8.11 release ? It has some useful function for Dell customers and > would avoid having to do special patches on top of the standard source > forge code. > > Tim Bell > CERN > > > ------------------------------------------------------------------------------ > _______________________________________________ > Ipmitool-devel mailing list > Ipmitool-devel@lists.sourceforge.net > https://*lists.sourceforge.net/lists/listinfo/ipmitool-devel > -- Albert Chu ch...@llnl.gov Computer Scientist High Performance Systems Division Lawrence Livermore National Laboratory ------------------------------------------------------------------------------ _______________________________________________ Ipmitool-devel mailing list Ipmitool-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
