Closed amsorrytola closed 6 months ago
Hey @mohammed-talha-ansari
Thanks for contributing and working on this repository. Can you also add some unit tests for the function you wrote to ensure that it is implemented correctly? You can refer the tests written in src/test
Thanks!
This change doesn't affect macro implementation due to the stack being the same but logically, macro definition should contain 4 instead of 0.
Then shouldn't it return 2 values as well? Also is this trend followed in all functions?
This change doesn't affect macro implementation due to the stack being the same but logically, macro definition should contain 4 instead of 0.
Then shouldn't it return 2 values as well? Also is this trend followed in all functions?
Yes, like I said what happens in the macro definition doesn't affect the actual working of the function but for clean and quality code writing, it is indeed a typo and should be takes(4) returns(2)
.
As for the other functions, I've checked that they don't have this typo.
This change doesn't affect macro implementation due to the stack being the same but logically, macro definition should contain 4 instead of 0.
Then shouldn't it return 2 values as well? Also is this trend followed in all functions?
Yes, like I said what happens in the macro definition doesn't affect the actual working of the function but for clean and quality code writing, it is indeed a typo and should be
takes(4) returns(2)
. As for the other functions, I've checked that they don't have this typo.
Okay. @mohammed-talha-ansari can you also make the above change as mentioned by Preeti in this PR?