-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update catch #22
base: master
Are you sure you want to change the base?
Update catch #22
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good jo on the first draft. Fix readme and other stuff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a subfolder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessarily, we can remove it if you prefer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove it then, trying to reduce files and folders.
g++ -std=c++14 -Werror -Wuninitialized -g -c test-unit/catch/catch_amalgamated.cpp -o build/catch_amalgamated.o | ||
|
||
# then run this command to compile and run the tests (you do not need to run the previous command again) | ||
g++ -std=c++14 -Werror -Wuninitialized -g build/catch_amalgamated.o test-unit/test.cpp -o build/test; ./build/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be fixed in main readme as well.
I updated the readme, makefile, and test.cpp files. |
This pull request simply updates the version of catch provided and provides a 2 command system for building catch separately and then running your tests.
More could be done to make this easier for students to work with, but through @Brian-Magnuson and I's discussion we realized that how much extra scaffolding we provide depends on what the vision for the Edugator templates are.
As it stands now, the changes provided do not change the workflows on each programming quiz's Canvas page. It imports into Clion as expected and now benefits from quicker building without extra user input, and students will still have to open the command line from VSCode and run the commands to test, which will also benefit from speed improvements.
However, it is very possible to improve this workflow further - Brian proposes adding a Makefile so that students with make installed will only have to run
make
and the commandline process will work as expected. My thought is that if we're including a Makefile, we might as well include a CMakeLists.txt instead (or in addition to the makefile) to enable the same workflow that the Github templates provide. However, this would unavoidably expand the scope of what's provided in the template.Before we make a final decision, this will be a draft PR to signal that it's not ready for merging. What are your thoughts @kapooramanpreet?