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

Travis: Build against multiple R versions #2281

Merged
merged 11 commits into from Feb 8, 2019

Conversation

infotroph
Copy link
Member

Description

  • Run Travis tests against three versions of R in parallel
  • Fold up successful log sections so it's easier to find what failed
  • Report time used by each make step (deps/install/document/check) within every package. Especially useful for debugging timed-out builds
  • make check now skips rebuilding documentation if env var $REBUILD_DOCS is false
  • General cleanup: Remove unused packages, delete some build hacks that have outlived their purpose, etc.

Motivation and Context

R versions

The major change here is a transition from testing against one R version (hopefully the one most users have) to defining a build matrix so that we can test in multiple environments at once. The build matrix concept could be extended in the future to include different OSes, Postgres versions, environment variables, etc, but for this PR the matrix is one-dimensional and contains three R versions: release, devel, and oldrel. These are aliases defined by Travis and currently point respectively to R 3.5, nightly, and 3.4, but versions are updated each release.

Opinions, please: I currently have the devel and oldrel builds marked as allow_failures, which means the build is always run on all three versions, but will report success as long as it finishes successfully on R-release. My rationale is that this will let us keep an eye on how it's doing in other versions but won't commit us to maintaining them. Is this what we want?

  • Pro: If R-devel introduces breaking changes we might want to defer fixes until it's clear the change will actually be released, and similarly if R-oldrel stops working we might want to reserve the right to say "OK, WONTFIX, PEcAn doesn't support that version anymore".
  • Con: Any build that's allowed to fail will likely keep failing and become harder to fix later, so if we do want to commit to supporting devel or oldrel then we should commit now and require that they pass for every merge.

Skip documentation

I've updated scripts/check_with_errors.R to recognize an environment variable $REBUILD_DOCS and pass its logical value to the document argument of devtools::check. This is primarily to save a few seconds per package on Travis, where documentation is always freshly-built at check time, but feel free to use it any other time make check fails to document/not document things the way you want.

Review Time Estimate

  • Immediately
  • Within one week
  • When possible

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the CHANGELOG.md.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@infotroph
Copy link
Member Author

@mdietze I may not have made the "opinions, please" section prominent enough. Should I take your approval to also be an endorsement of allowing failures in oldrel and devel?

@mdietze
Copy link
Member

mdietze commented Feb 8, 2019

Yep, I endorse your approach, but didn’t pull yet so as to give others time to comment. Given how some packages change, it may be impossible to support multiple versions without code that detects which version is installed and switches among different code blocks, and that could become a nightmare

Copy link
Member

@robkooper robkooper 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, like the ability to compile against 3 versions of R. I agree with the OK to fail if old or next version are broken.

@mdietze mdietze merged commit 089d9ac into PecanProject:develop Feb 8, 2019
@infotroph infotroph deleted the travis-multiR branch February 13, 2019 07:09
@robkooper robkooper mentioned this pull request Sep 5, 2019
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.

None yet

3 participants