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 optional test building #141

Merged
merged 2 commits into from
Nov 10, 2019
Merged

Add optional test building #141

merged 2 commits into from
Nov 10, 2019

Conversation

robertohueso
Copy link
Member

According to CMakeLists.txt

This project has no configurable options---it just installs the headers to the install location, and optionally builds the test program.

but it's not optional at the moment. This PR adds the option in CMake 😄.

Add a new option to choose whether to build tests or not.
@mlpack-bot
Copy link

mlpack-bot bot commented Nov 9, 2019

Thanks for opening your first pull request in this repository! Someone will review it when they have a chance. In the mean time, please be sure that you've handled the following things, to make the review process quicker and easier:

  • All code should follow the style guide
  • Documentation added for any new functionality
  • Tests added for any new functionality
  • Tests that are added follow the testing guide
  • Headers and license information added to the top of any new code files
  • HISTORY.md updated if the changes are big or user-facing
  • All CI checks should be passing

Thank you again for your contributions! 👍

Copy link
Member

@rcurtin rcurtin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks for fixing that documentation discrepancy. :)

Do you want to also add a note to HISTORY.md? If not I can handle it during merge. 👍

@robertohueso
Copy link
Member Author

History updated :) I forgot it 😅.

Copy link
Member

@zoq zoq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@mlpack-bot
Copy link

mlpack-bot bot commented Nov 10, 2019

Hello there! Thanks for your contribution. I see that this is your first contribution to mlpack. If you'd like to add your name to the list of contributors in src/mlpack/core.hpp and COPYRIGHT.txt and you haven't already, please feel free to push a change to this PR---or, if it gets merged before you can, feel free to open another PR.

In addition, if you'd like some stickers to put on your laptop, I'd be happy to help get them in the mail for you. Just send an email with your physical mailing address to stickers@mlpack.org, and then one of the mlpack maintainers will put some stickers in an envelope for you. It may take a few weeks to get them, depending on your location. 👍

@zoq zoq merged commit 91e5abb into mlpack:master Nov 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants