>> I guess we can still refactor later, it's just one >> thing to thing about when introducing an API >> that will likely be used a lot down the road. > > I'm not sure what we want right now, hence why I left it a little more > vague. At this point in time all the relevant callers I can think of > (or rather potential callers) don't care about the failure and just want > to know if it succeeded. I think it would be ok to do a small refactor > at a later time if we really needed to provide the reason for the > failure. Unless of course someone feels strongly enough that it needs > to be addressed right now. If we did address it now then we would need > a group of #define's or maybe an enum to describe the failure modes. I do not feel strongly, just wanted to draw your attention to it. And having thought about it, refactoring down the road is likely quite cheap, so this was a useless bikeshedding attempt. >> >> I applaud the effort towards documenting what each variable is >> supposed to contain. But some of them read like >> >> /* increments i by one */ >> i++; >> >> which is considered bad comment style (it doesn't add >> more information, it just wastes a line), so specifically for >> all the "Path to X" comments: >> * Are they absolute path, or relative path? >> If relative, then relative to what? >> * Can they be NULL? When? >> >> (* Why do we need so many path? >> Could one of them be constructed using >> another and then hardcoding a string relative to it? >> This question may rather be answered in the commit >> message) > > Thanks for pointing this out. I'll work a little bit more on the > comments to be more descriptive. I do think that all field names should > probably be commented though. Thanks! Stefan
