cmu-cs-academy / desktop-cmu-graphics

BSD 3-Clause "New" or "Revised" License
15 stars 9 forks source link

Add docs explaining how to run the tests #78

Closed schmave closed 2 months ago

Mohamed-Waiel-Shikfa commented 2 months ago

Thank you for the valuable information. That being said I tried following the steps and the build failed before getting to testing. I troubleshooted a bit and found out that I had to install cmake. So I did, but the build still failed before getting to testing for a different reason. What I am trying to say is that this document could be improved by adding a getting started section that explains the correct way to clone the repo for easy editing and testing (maybe by using the --editable flag of pip), where to setup the venv, what tools and program are needed (python, cmake...), and any other useful information to getting started. There are also two other section that can be useful although less important than the above: 1 - a section on how to push changes 2 - a quick outline of the project, its structure and how it works / comes together

schmave commented 2 months ago

Hi Mohamed, thanks for the message. I appreciate your suggestions about additional information to add. I agree that that would be helpful. Unfortunately, we have very limited time to work on this library so it may be a while before we get to that.

What operating system are you using to run the tests? Also if you can provide more info about what failed that required you to install cmake that would be helpful for improving the instructions and getting you to be able to run the tests locally. A full log showing all output from the commands you ran would be great.

For what it's worth, this Appveyor config file has all the steps that we use to run the tests on Mac and Windows: https://github.com/cmu-cs-academy/desktop-cmu-graphics/blob/main/.appveyor.yml

Mohamed-Waiel-Shikfa commented 2 months ago

2 Days later and the info provided turned out to be very useful, that being said I have some info that I will add as part of a new PR.

Thanks