Closed DeveloperPaul123 closed 6 years ago
Yes, please go for it. And in the same PR, please add some documentation as a guideline for the other developers. It would be nice to start to develop the code in a coherent way.
All done, let me know if you have questions.
This issue has been addressed in #23. Closing.
I see that the PR is still not merged. Might it be better to leave this issue open until the code is actually merged into the repo?
I prefer to keep it close now to prevent other people from opening new pull requests on this. I will check your PR asap for merging. In case we need it, we can reopen the issue later.
Il 16/Ott/2018 02:53 AM, "Paul T" notifications@github.com ha scritto:
I see that the PR is still not merged. Might it be better to leave this issue open until the code is actually merged into the repo?
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/mattiadg/BoardGames/issues/21#issuecomment-430063428, or mute the thread https://github.com/notifications/unsubscribe-auth/AD6xol3ZEAgkuBLMq4PFerD5OuPTdmTAks5ulS4DgaJpZM4XbT6t .
I think your comment indicates that this issue has been addressed but that works too. Thanks!
No, it means that I want to merge your PR, but I didn't have time to check it. In the meanwhile, I don't want that other people see the issue open and submit new PRs for the same issue.
Since this project seems to be geared towards learning, it may be good to make it fully cross platform and build tool chain independent. Adding CMake support would fix this. Let me know if you're open to this and I can file a PR.
Note that adding CMake support may make easier to integrate other libraries with tools like
vcpkg
which could also facilitate adding unit tests with a framework of choice (vcpkg
supports bothcatch2
andgoogletest
).