Skip to content

backport trimws #3114

Merged
mattdowle merged 1 commit intomasterfrom
trimws_backport
Oct 18, 2018
Merged

backport trimws #3114
mattdowle merged 1 commit intomasterfrom
trimws_backport

Conversation

@mattdowle
Copy link
Copy Markdown
Member

@mattdowle mattdowle commented Oct 18, 2018

Currently failing tests under R 3.1.0, thanks to extended-CI.
(trimws is used in ci.R and publish.R too but those scripts are in root only not the package.)

@mattdowle mattdowle added this to the 1.12.0 milestone Oct 18, 2018
Comment thread R/data.table.R
@@ -1,4 +1,12 @@

if (!exists("trimws", "package:base")) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

better as

with(R.version, sprintf('%s.%s', major, minor)) >= '3.2.0'

?

Copy link
Copy Markdown
Member Author

@mattdowle mattdowle Oct 18, 2018

Choose a reason for hiding this comment

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

You mean if (getRversion() >= '3.2.0') I think. The comparison method for the package_version class does the right thing. But see comments in onLoad.R about why we don't switch on version number, but functionality instead. So much so that getRversion() is redefined to be stop() internally as a reminder.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

good to know!

@mattdowle mattdowle merged commit 6419f8a into master Oct 18, 2018
@mattdowle mattdowle deleted the trimws_backport branch October 18, 2018 05:26
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.

2 participants