Bug #3070
closedBlock::remove unexpected semantics
100%
Description
Block
class has a remove
method:
void
remove(uint32_t type);
This method is missing Doxygen, is missing a test case, and is unused within ndn-cxx.
Based on function name and signature, either of the following semantics are reasonable:
/** \brief remove all subelements of \p type
* \param type TLV-TYPE of subelement to remove
* \pre parse() has been invoked
*/
/** \brief remove the first subelements of \p type
* \param type TLV-TYPE of subelement to remove
* \pre parse() has been invoked
*/
However, the actual behavior is: remove all subelements whose TLV-TYPE is not type
.
This semantics is surprising.
To solve this bug:
- decide on the correct semantics
- add Doxygen
- add test case
- fix implementation
- post a "potential breaking change" notice to ndn-lib mailing list
- fix regressions on dependent projects, if any
Updated by Junxiao Shi over 9 years ago
- Blocks Feature #2879: NDNLPv2: Packet and Fields added
Updated by Junxiao Shi over 9 years ago
- Assignee set to Eric Newberry
I prefer the first semantics, ie. remove all subelements of type
, because std::multiset::erase(key)
also removes all elements.
Updated by Eric Newberry over 9 years ago
- Status changed from New to In Progress
Updated by Eric Newberry over 9 years ago
I agree with note-2 and I plan to implement it this way.
Updated by Eric Newberry over 9 years ago
- Status changed from In Progress to Code review
Updated by Eric Newberry over 9 years ago
How would we do the announcement to the mailing list when the time comes? Is there any particular message that needs to be included>
Updated by Junxiao Shi over 9 years ago
How would we do the announcement to the mailing list when the time comes? Is there any particular message that needs to be included?
There's no particular requirement on what to include in the announcement.
One example is http://www.lists.cs.ucla.edu/pipermail/ndn-lib/2014-November/000211.html
Updated by Eric Newberry over 9 years ago
In the message to the ndn-lib group, should I mention that the change has already been merged?
Updated by Eric Newberry over 9 years ago
Please disregard note 8.
The message has been sent to ndn-lib.
Updated by Junxiao Shi over 9 years ago
- Status changed from Code review to Closed
- % Done changed from 0 to 100