justincpresley / ndn-python-svs

ndn-python-svs: NDN StateVectorSync protocol python library for syncing distributed real-time applications.
GNU Lesser General Public License v2.1
9 stars 5 forks source link

Add test cases for encoding/decoding a state vector #3

Closed phylib closed 3 years ago

phylib commented 3 years ago

Hi @justincpresley, in your test case, you had some StateVector generation before actually parsing the TLV. I did not see a reason for that. I removed that and added a test case to check if generated TLVs also comply with the standard.

phylib commented 3 years ago

Hi @justincpresley, can you explain why you removed the source code comments with #2af3a18? In general I would recommend adding more documentation, eg. by using Docstring.

justincpresley commented 3 years ago

Hey @phylib, thank you for asking!

I do agree with you on the more documentation part. For now, major of the code is separated enough to be at understandable with the current documentation. I am not a big fan of Docstring (I have worked on many projects with it) and since I will be the major one working on this project (with security portion), I find it easier to not incorporate the Docstring way of documenting, at least not yet.

However, I took out the source code comments (for the purposes of merging) to keep the same documenting style throughout this github repo. I have noted the comments you provided though.

Feel free to add your comments back in your fork to make it clearer.

phylib commented 3 years ago

I have a couple of comments to your reply: