CC: undisclosed-recipients: ; MIME-Version: 1.0 In-Reply-To: Content-Disposition: inline References: Message-ID: <20140417205234.GA17567@isc.org> Content-Type: text/plain; charset="utf-8" X-RT-Original-Encoding: utf-8 Received: from bikeshed.isc.org (bikeshed.isc.org [149.20.48.19]) by bugs.isc.org (Postfix) with ESMTP id 639692D20571 for ; Thu, 17 Apr 2014 20:52:34 +0000 (UTC) Received: by bikeshed.isc.org (Postfix, from userid 10292) id 4A58E216C3B; Thu, 17 Apr 2014 20:52:34 +0000 (UTC) Delivered-To: bind9-bugs@bugs.isc.org User-Agent: Mutt/1.4.2.3i Subject: Re: [ISC-Bugs #35793] [PATCH] Date-format serial numbers Return-Path: X-Original-To: bind9-bugs@bugs.isc.org Date: Thu, 17 Apr 2014 20:52:34 +0000 To: Bradley Forschinger via RT From: Evan Hunt RT-Message-ID: Content-Length: 1097 Thank you Bradley! Quite well done, both in terms of style and completeness; I particular appreciate your adding both ATF and system tests. Things I changed during review: - the command syntax "yyyymmddvv" seemed likely to be hard to remember, so I just replaced it with "date". - the unit tests weren't quite effective as written: the "old" serial number was being set incorrectly as a unixtime value, so the "new" serial never came into conflict with it. changing to, for instance: /* future to date */ set_mystdtime(2014, 4, 2); old = dns_update_soaserial(0, dns_updatemethod_date); set_mystdtime(2014, 4, 1); ... enables the test to correctly check the behavior of a serial number going forward or backward in time. - the julian date converter you used in epoch_to_yyyymmdd() is nifty, but a) I'm not capable of evaluating its correctness and b) it's inconsistent with the way we've done similar conversions elsewhere in BIND, so I switched it to use gmtime(). - there was no documentation; I added a paragraph to doc/arm/Bv9ARM-book.xml