Open
Conversation
Herschenglime
commented
Feb 9, 2024
kapooramanpreet
requested changes
Feb 9, 2024
Member
kapooramanpreet
left a comment
kapooramanpreet
left a comment
There was a problem hiding this comment.
Good jo on the first draft. Fix readme and other stuff.
Author
There was a problem hiding this comment.
Not necessarily, we can remove it if you prefer
Member
There was a problem hiding this comment.
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 |
Member
There was a problem hiding this comment.
this should be fixed in main readme as well.
|
I updated the readme, makefile, and test.cpp files. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
makeand 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?