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] Restructure Testing chapter #613

Closed
wants to merge 16 commits into from

Conversation

rosiehigman
Copy link
Collaborator

Summary

Addresses (but does not close) #468

Reformatting to put a new sentence on each line and other similar small fixes

List of changes proposed in this PR (pull-request)

  • Lorem ipsum dolor sit amet, consectetur adipiscing.
  • Lorem ipsum dolor sit amet, consectetur adipiscing.

What should a reviewer concentrate their feedback on?

  • Lorem ipsum dolor sit amet, consectetur adipiscing.
  • Everything looks ok?

Acknowledging contributors

@netlify
Copy link

netlify bot commented May 28, 2019

Deploy preview for the-turing-way ready!

Built with commit 0a416af

https://deploy-preview-613--the-turing-way.netlify.app

@rainsworth rainsworth added the book-dash-ldn2019 Issues related to contributions made during the London Book Dash label May 28, 2019
Also moved each sentence onto a new line and took out several latin abbreviations
Also separated sentences out onto new lines
Also split each sentence onto a new line
Comment on lines +82 to +84
- R unit-tests
- RUnit
- svUnit (works with SciViews GUI)
Copy link
Contributor

@OriolAbril OriolAbril Nov 20, 2019

Choose a reason for hiding this comment

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

I would add testthat and tinytest to the list and remove RUnit or say it is not recommended.

testthat is by far the most popular package for R (actually the example of asserting an equality below is done with testthat) and is actively developed. tinytest is also being actively developed whereas RUnit's last release is more than a year old, which triggered some packages to stop using it (i.e. RcppArmadillo).

I was thinking on adding a simple PR for this alone, but I don't want to step on your toes nor difficult this amazing refactoring, so I am fine with you adding the changes directly here if it makes your life easier.

Copy link
Collaborator

@KirstieJane KirstieJane Nov 20, 2019

Choose a reason for hiding this comment

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

Thanks @OriolAbril!

I think this pull request is a little stale (@rosiehigman has a new job!) so I'm not sure who is going to pick this up, so if you wanted to do a little PR then please go for it, but otherwise just leave this comment here so someone can find it again in the future!

@annakrystalli - just tagging you in case you have strong opinions about R testing (I don't know anything John Snow!) @OriolAbril's reasoning makes total sense to me 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

I opened #760 to change these 2 lines

@masonlr masonlr closed this Aug 1, 2022
Outstanding Issues + Pull Requests Review automation moved this from Outstanding PRs to Done Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
book-dash-ldn2019 Issues related to contributions made during the London Book Dash
Development

Successfully merging this pull request may close these issues.

None yet

5 participants