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

WIP: Remove Boost as a required dependency #159

Closed
wants to merge 7 commits into from
Closed

Conversation

arcuru
Copy link
Contributor

@arcuru arcuru commented Oct 29, 2017

After one too many issues with people having trouble installing Boost, I created this to get feedback about removing it as a required dependency, primarily by switching to a header-only unit testing suite.

This closes every open bug in the repo (adding the tags here for auto-closing)
(closes #96, closes #116, closes #123, closes #126, closes #149, closes #150)

The primary goal was to remove Boost as a default dependency, though some tests might still require it. This change removes it from everywhere except for the meetup test, as that's already a nightmare of a test.

This moves to using Catch for testing, because as far as I can tell that is the most popular header-only unit testing suite for C++ (I have no stake in this, I've never used it before). This allows us to bundle the test suite along with the problems, so there are no dependencies other than a C++11 compliant compiler and some toolchain (CMake + Make, or almost any IDE can work with the files directly).

For the majority of problems, this just involved replacing the Boost.Test macros with their Catch equivalents, but a handful of problems have more substantial changes:

  • gigasecond is the most heavily impacted test, and is a breaking change for existing solutions. It used boost::posix_time across the user interface so I rewrote the interface to use strings, the example to use <ctime>, and increased the difficulty. It should at some point be re-ordered later in the track.
  • robot-names used Boost.Regex to validate the user supplied names in the test suite. I attempted to replace this with std::regex, but due to issues on Travis-CI (that I wasn't able to reproduce with the same toolchain outside Travis) I just rewrote the validation to avoid regex. Ideally this should just use std::regex, but I didn't want to spend more time debugging it for a test that's just a one-liner if statement. See 1ff8dba
  • anagram, bob, crypto-square, and word-count all used Boost libs in the example solutions (that was not user visible in the test suites). I removed those in change 8922c1f, but I don't consider those changes to be too important, since they are not user visible.

Documentation has not yet been updated, but will be if there's support for this change.

@DonBarredora
Copy link

Is this ready to be merged into the master branch?

@LegalizeAdulthood
Copy link
Contributor

I think it would need to coincide with a big docs update. Is that correct, Patrick?

@pacmancoder
Copy link

pacmancoder commented Jul 4, 2018

Bump. @LegalizeAdulthood, @jackhughesweb

I think this issue should solved, as exercism should teach how to use C++ according to its modern standard. I think that all code, which use Boost should be replaced with corresponding stl-based.

I can dig into this issue and update exercises.
By the way, I think Gigasecond should use modern std::chrono, not ctime after refactoring.

So, how can I help with this issue? @patricksjackson, can I cooperate with you somehow to continue work on this PR?

@arcuru
Copy link
Contributor Author

arcuru commented Aug 23, 2018

Sorry for the delay here, I assumed somebody would've finished this change without me in the past yearish this has been open. I had to step away for a little while for various reasons, but I have a little time now to help out and clean up around here.

I will be taking another look at this and updating it for merge over the next few weeks.

@pacmancoder or others, if you would like to help please open a PR against this branch. (This is tracked as another branch in this repo, so you can checkout this branch and make a PR targeting it. If not you can comment here with a pointer to your change and I can pull the commit).

This will be a big change, as all the Docs need to be updated as well. I will probably need help for testing it at the very least. I'll update here and/or make a tracking issue to track everything that needs to happen and how you could help.

@arcuru arcuru self-assigned this Feb 26, 2019
@arcuru arcuru changed the title Remove Boost as a required dependency WIP: Remove Boost as a required dependency Feb 26, 2019
@arcuru arcuru mentioned this pull request Mar 17, 2019
6 tasks
@arcuru
Copy link
Contributor Author

arcuru commented Mar 17, 2019

Deprecating this, using #233 to track the work needed to incrementally move to Catch.

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