Skip to content

Conversation

yutannihilation
Copy link
Member

@yutannihilation yutannihilation commented Apr 25, 2020

Now R 4.0.0 is released, let's drop support for R 3.2 (c.f. we support only 4 previous versions of R https://www.tidyverse.org/blog/2019/04/r-version-support/). let's add R 4.0 runners on CI

In addition, this PR contains the following tweaks:

@yutannihilation yutannihilation added this to the ggplot2 3.3.1 milestone Apr 25, 2020
@jimhester
Copy link
Contributor

I generally think it is better to be explicit with what versions you are checking rather than using the aliases.

One reason is that you will have to bump the cache numbers to invalidate the cache when the versions change anyway, so if you have to edit the yaml anyway might as well make it very clear what versions you are checking against.

@yutannihilation
Copy link
Member Author

Thanks! It makes sense. I was wondering why R-CMD-check.yaml refers to the matrix.config.r instead of the actual R version (e.g. we can include the version in depends.Rds), but now I understand the design.

@yutannihilation yutannihilation changed the title WIP: Drop support for R 3.2 Drop support for R 3.2 Apr 27, 2020
Copy link
Member

@clauswilke clauswilke left a comment

Choose a reason for hiding this comment

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

This looks fine to me but I don't know whether I'm the right person to review. Questions to which I don't know the answer:

  1. Do we want to change the R version requirement for an upcoming minor release (presumably ggplot2 3.3.1 should come out soon)?

  2. Do we always increase the version in Depends when we drop supported versions? As far as I can tell, there's no change in the code that causes ggplot2 to break for R 3.2, we're just not checking anymore.

@yutannihilation
Copy link
Member Author

Aw, true. Sorry, probably I should have asked Hadley and Thomas.

1. Do we want to change the R version requirement for an upcoming minor release (presumably ggplot2 3.3.1 should come out soon)?

Yes, since the second next release (currently expected as "v3.4.0") should be distant, I thought we should be clear about dropping the support in the next version to prevent confusion.

2. Do we always increase the version in Depends when we drop supported versions? As far as I can tell, there's no change in the code that causes ggplot2 to break for R 3.2, we're just not checking anymore.

Yes, I think we are supposed to increase it even if there's no actual breakage, but I might misunderstand the rule of tidyverse.

@hadley @thomasp85
As R 4.0.0 is released, all tidyverse packages are going to bump the R version requirement, right? If so, I want to include it in the upcoming v3.3.1. Does it sound good to you? Or, do we need some strategy about the release schedules?

@jimhester
Copy link
Contributor

No, you don't need to bump the minor version of the required version of R unless there is something you use in ggplot2 that would require it.

@yutannihilation
Copy link
Member Author

Oh, sorry. I misunderstood the process... Then, should I keep the R 3.2 runner to know when we actually need to drop the support?

@hadley
Copy link
Member

hadley commented Apr 29, 2020

@yutannihilation Yes. Generally what we do is keep the dependency and runners as is (just adding a new one for the latest version), and then only drop when we have to (i.e. when you use a function or package that needs 3.3). We should really document this process somewhere 😬

@yutannihilation
Copy link
Member Author

@hadley
OK, thanks for clarifying! I'll do this better next year...

@clauswilke
Sorry for confusing you. I'll restore the minimum requirement and the CI runners.

@yutannihilation yutannihilation changed the title Drop support for R 3.2 Test R 4.0 on CI Apr 29, 2020
@yutannihilation yutannihilation merged commit 818cf80 into tidyverse:master Apr 29, 2020
@yutannihilation yutannihilation deleted the ci/bump-r-version-to-4.0 branch April 29, 2020 14:28
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.

4 participants