You are viewing a single comment's thread from:
RE: ChatChain: A ChatServices linking API
Thank you for your contribution. Despiste you are working on your branch rewrite which hasn't been merged yet to the master branch, I would like to give a few cents of my review.
- I notice that your commit messages are sort-of
testingXXX- which doesn't really give much information. - I haven't see any existing unit tests, nor the new tests added while you make changes - which is a bit concern.
- Code not needed should really be deleted instead of commenting-out. For example, here you commented out code, which is hard to understand without comments and especially the commit messages are not descriptive.
Your contribution has been evaluated according to Utopian policies and guidelines, as well as a predefined set of questions pertaining to the category.
To view those questions and the relevant answers related to your post, click here.
Need help? Chat with us on Discord.
Hi there, thanks a tonne for your input, currently i've been trying mostly to get to a release point before i go through and cleanup all my code (which would be when i PR the rewrite branch in to the Master branch).
One of the things that i've never quite understood is Unit Testing, i'm mostly self taught for the past 4/5 years and my testing is always done by actually attempting to use the service/program instead of using these. in my personal opinion Unit testing is generally more of a preference rather than a need.
Yes i really need to remove those blocks of commented out code, i'm actually going to do that right now
Also on that note, the names of the commits i'm quite aware are horrible, the only reason i am committing them is to push them to our build system so i can test them easily. and with the amount of times i need to do that, i generally can't be bothered naming them.
but i definitely will be properly naming them once i get to a release point and need a lot less commits for testing
I did a cleanup for ya, not everything i want to cleanup, but that'll be done when i'm ready to merge it into Master (i'd like to go through and document everything before i merge)
https://github.com/ldtteam/ChatChain/commit/d33bf98e75923062e3f76a4fa8e0002f8c095e11
Thank you for your review, @justyy! Keep up the good work!