Opened 10 years ago

Closed 10 years ago

#53 closed task (fixed)

review: Question class (DNS message API)

Reported by: jinmei Owned by: jinmei
Priority: very high Milestone: 02. Running, functional authoritative-only server
Component: libdns++ Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: Add Hours to Ticket:
Total Hours: Internal?:

Description

It's NOT yet ready for review, but I created the ticket to record the
beginning revision on the trunk for this work.

It's rev 941.

Subtickets

Change History (6)

comment:1 Changed 10 years ago by jinmei

  • Component changed from Unclassified to DNSPacket API
  • Milestone set to 02. Running, functional authoritative-only server
  • Owner set to jinmei
  • Status changed from new to assigned
  • Type changed from defect to task

comment:2 Changed 10 years ago by jinmei

  • Priority changed from major to blocker

The question class of the DNS message API is ready for review.

The corresponding diff is between rev 941 and 944, but this is
basically a new file, so it would be easier to simply review the
following files as a whole:

  • question.h
  • question.cc
  • question_unittest.cc (and the associated data files)

Unittest covers 100% of the code (according to lcov). lcov results
are available at: http://bind10.isc.org/~jinmei/lcov-question/

HTML version of doxygen document can be seen at:
http://bind10.isc.org/~jinmei/doc-question/classisc_1_1dns_1_1_question.html

BTW: the implementation is quite simple and should be easy to review.

comment:3 Changed 10 years ago by jinmei

  • Owner jinmei deleted

comment:4 Changed 10 years ago by jinmei

  • Owner set to jinmei
  • Status changed from assigned to accepted

comment:5 follow-up: Changed 10 years ago by each

It looks fine. I'm curious why you decided to remove the == and != operators, though?

comment:6 in reply to: ↑ 5 Changed 10 years ago by jinmei

  • Resolution set to fixed
  • Status changed from accepted to closed

Replying to each:

It looks fine. I'm curious why you decided to remove the == and !=
operators, though?

Thanks for the review.

Regarding == and !=: They were actually unnecessary but added for gtest (EXPECT_EQ
required it) as a quick hack.

We could keep them, but in general I want to make the whole library as
simple as possible, and tried to avoid adding features of "we don't
need it now but might be useful in the future". If we have a
practical need for them it's easy to add them again.

Closing the ticket.

Note: See TracTickets for help on using tickets.