Closed Kashish0423 closed 3 years ago
Amazing code other than these little changes, Ready to be merged, just resolve these little things quickly @Kashish0423
Also, the tests and documentation have not been added for the changes. Tests are in the
tests
folder at root of the project, and Swagger JSDocs are inroutes/opportunity.js
,As a rule of thumb, tests and Documentations should always be updated while making a PR. Please do that also.
Hello mam
In the Tests folders , I can see 3 folders managers, controllers and services , so where do I need to add test cases , also the Test cases that I am thinking about are like the cases that I try and test the code eg limit=2 and page=3 and something like that but in the file I see some code like using functions (expect() , stub(), spy() )etc for the very first time so I don't know at the moment how to solve it , if possible can you assign it as another issue as I would have to go in to the concepts of the code in order to implement it and the issue will remain pending for more time .
As far documentation is concerned , do I have to change it in the apis.md file as well ?
Amazing code other than these little changes, Ready to be merged, just resolve these little things quickly @Kashish0423 Also, the tests and documentation have not been added for the changes. Tests are in the
tests
folder at root of the project, and Swagger JSDocs are inroutes/opportunity.js
, As a rule of thumb, tests and Documentations should always be updated while making a PR. Please do that also.Hello mam
- In the Tests folders , I can see 3 folders managers, controllers and services , so where do I need to add test cases , also the Test cases that I am thinking about are like the cases that I try and test the code eg limit=2 and page=3 and something like that but in the file I see some code like using functions (expect() , stub(), spy() )etc for the very first time so I don't know at the moment how to solve it , if possible can you assign it as another issue as I would have to go in to the concepts of the code in order to implement it and the issue will remain pending for more time .
- As far documentation is concerned , do I have to change it in the apis.md file as well ?
routes/opportunity.js
, open that file, (pull the latest code if you can't see the documentation there), you have to edit the get API
Documentation there. It's fairly simple, you will understand it by looking at the already existing documentation. Hello @Manvityagi mam please check my latest commit and as of now the branch doesn't have any conflicts and it also passed all the checks , I have made all the necessary changes .
Fixed #44
Hello @Manvityagi , I had conflicts in the previous PR, i was facing some issues to solve them and wasn't able to identify how to get out of it, so I made a new branch just to reduce the time of getting the work done , please have a look at it and let me know if there are any changes that should be made .