On Tue, 10 Apr 2012, Adam Wolfe Gordon <[email protected]> wrote: > Hi Mark, > > This looks good to me, but I haven't tested it. It's probably worth > adding a test for the new functionality.
Yes I agree. I could add a test to the json tests, or I could update the pair of test patches at id:"[email protected]" as there do not seem to be any tests for --entire-thread or not for the other formats either. What do you think? > One style issue below, which is a matter of taste and I'll defer to > others if they disagree: > > On Tue, Apr 10, 2012 at 11:04, Mark Walters <[email protected]> wrote: >> @@ -895,10 +905,11 @@ show_messages (void *ctx, >> if (status && !res) >> res = status; >> next_indent = indent + 1; >> + } else >> + status = show_null_message (format); > > I accept, but don't particularly like, the notmuch style of omitting > braces where they aren't required. However, an else with a brace on > only one side just looks weird. If they're like this everywhere else > then I guess it's best to be consistent, but to me it's a lot more > readable as } else {. > > As I said above, I'll defer to others' judgement here, just thought it > was worth pointing out. Yes I will fix this in the next version (I recall Jani agreed with you so I will take that as preferred unless someone says otherwise.) thanks Mark _______________________________________________ notmuch mailing list [email protected] http://notmuchmail.org/mailman/listinfo/notmuch
