BIND 10 #1976: use meta-or-container-of data source in b10-auth
BIND 10 Development
do-not-reply at isc.org
Fri Jul 6 00:59:59 UTC 2012
#1976: use meta-or-container-of data source in b10-auth
-------------------------------------+-------------------------------------
Reporter: | Owner: vorner
jinmei | Status: reviewing
Type: task | Milestone:
Priority: | Sprint-20120717
medium | Resolution:
Component: | Sensitive: 0
b10-auth | Sub-Project: DNS
Keywords: | Estimated Difficulty: 8
Defect Severity: N/A | Total Hours: 0
Feature Depending on Ticket: |
Add Hours to Ticket: 0 |
Internal?: 0 |
-------------------------------------+-------------------------------------
Changes (by muks):
* owner: muks => vorner
Comment:
Hi vorner
The following are my review comments. I have read the rest and have not
found anything could be an issue. However, as I'm looking at a lot of this
code for the first time as well, I suggest that someone else like Jinmei
also review this branch. I have reviewed up to commit
`af4e7787d37b951f9a48e992f0e2c6f4e4a28c22` ([1976] Add missing test file).
* There's a spelling mistake here:
{{{
+++ b/src/bin/auth/datasrc_configurator.h
+ /// is unkown and it would be questionable at least). It is called
}}}
* Can you rename `DataSourceConfiguratorGeneric::deinit()` to `cleanup()`?
* Can only one AuthSrv instance use `DataSourceConfigurator`, if `init()`
is called with it?
* Why is `DataSourceConfiguratorGeneric::reconfigure()` public?
* Can `doInInit()` be renamed to something else so it reads clearly, such
as `initializeList()`?
* This comment is incorrect in `DatasrcConfiguratorTest::multiple()`:
{{{
+ // This one does not set
+ EXPECT_EQ("get CH\nset CH xxx\nget IN\nset IN yyy\n", log_);
}}}
* Similarly in `DatasrcConfiguratorTest::updateAdd()` too
* It's probably good to have `lists_.size()` check in
`DatasrcConfiguratorTest::createList()` and
`DatasrcConfiguratorTest::modifyList()` too
* You should add `lists_.size()` check to
DatasrcConfiguratorTest::updateDelete()
* Can you change this to read "Can't reconfigure when not initialized with
init()" in `reconfigure()`:
{{{
+ if (server_ == NULL) {
+ isc_throw(isc::InvalidOperation,
+ "Can't reconfigure while not inited");
+ }
}}}
--
Ticket URL: <http://bind10.isc.org/ticket/1976#comment:8>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list