Skip to content
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

Add CMake compatibility, cleanup/improve project structure #35

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

CodeFinder2
Copy link

Hi,

thanks for this library! :-)

I've added CMake support in order to integrate it in my (CMake) project using CPM.

Along the way, I've also revisited the project's directory structure to more adhere to modern C++ project structures, especially placing headers in include/$name_of_the_project/ to avoid conflicts with equally named headers of other projects (if any).

Would be glad if you could take a look and merge this! :-)

PS: I also updated your Makefile but it fails with

g++ base64-17.o test-17.o -o base64-test-17
base64-test-11
make: base64-test-11: command not found
make: *** [Makefile:30: test] error 127

However, I already got this error before (after just cloning your repo) so it should not be an issue caused by my modifications.

@ntwerdochlib
Copy link

Regarding the command not found error, I would update the Makefile as follows:

diff --git a/Makefile b/Makefile
index 126e2ca..0ac27ed 100644
--- a/Makefile
+++ b/Makefile
@@ -27,8 +27,8 @@ WARNINGS=                    \
    -fdiagnostics-show-option

 test: base64-test-11 base64-test-17
-       base64-test-11
-       base64-test-17
+       -./base64-test-11
+       -./base64-test-17

 base64-test-11: base64-11.o test-11.o
        g++ base64-11.o test-11.o -o $@

which instructs the test recipe to run the binaries from the local directory and ignore any returned error, so both tests run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants