On Thu, 02 Jun 2011 10:49:57 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote: > Well, it says that changes are in notmuch 0.5. So "old" and "previous" > refer to pre-0.5 (i.e. 0.4) and "new" refers to 0.5. Sure, but I happen to ahve already forgotten the details of how the variable could be configured in 0.4 and in 0.5. More importantly, anyone in the future reading the commit log is much more likely not to remember. > Any configuration when `notmuch-fcc-dirs' is a list. That variable has > a nice documentation. Again, I'd like our commit messages to be self-contained. They are much more useful if the describe the change being made without assuming to much outside knowledge. > > It would be easier to understand the code if there were a corresponding > > test case for it. ... > I do not think we need a test for this fix. What we need are tests for > FCC functionality when notmuch-fcc-dirs is a list. Yes! > Old configuration format was changed in 0.5 in an incompatible way. > There is a check for the unsupported old-style configuration. But the > check is broken and results in an error when running with a valid > new-style configuration. This is actually what I meant by "corresponding test case". If the bug here is that a "new-style configuration" doesn't work , (and I still don't like that wording---don't say "new style"---explain what it actually *is*), then yes, we need a test case showing that bug. > I am not sure what you expect from the commit message here. IMO it is > enough for this small bugfix and those who interested can always refer > to documentation for details. The commit message should provide a self-contained description of the change. It should be along the lines of: When fcc-dirs is set to <some-particular-datatype-that-should-work> notmuch was incorecctly detecting this as the <old-style-that-is-no-longer-supported> and generating an error message. Fix the test so that this configuration now works. Where the <phrases> above should be replaced with actual descriptions, not relative pointers to information like "old style" or "new style". Does that make sense? -Carl