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

use_tidy_versions should warn when the local version is not available on CRAN #309

Closed
romainfrancois opened this issue Mar 12, 2018 · 10 comments
Closed

Comments

@romainfrancois
Copy link
Contributor

@romainfrancois romainfrancois commented Mar 12, 2018

and then perhaps advise to add a Remote.

This is sort of related to #179

This is e.g. the reason for this commit: tidyverse/dplyr@ac722d1

@jennybc jennybc added this to the v1.4.0 milestone May 25, 2018
@jennybc
Copy link
Member

@jennybc jennybc commented May 25, 2018

@hadley How do you want me to handle the situation when local version is ahead of CRAN?

@hadley
Copy link
Member

@hadley hadley commented May 28, 2018

I think use_tidy_versions() should probably use CRAN versions for everything, but not replace any versions that are higher than the CRAN version (so you can still manually require a specific dev version if needed)

@jennybc
Copy link
Member

@jennybc jennybc commented Jun 1, 2018

For concrete discussion, here's the current state of usethis deps that are not R itself or base or recommended packages, on my machine:

> deps
       type    package        version   cran       local
2   Imports  backports       >= 1.1.0  1.1.2       1.1.2
3   Imports      clipr       >= 0.3.0  0.4.0       0.4.0
4   Imports clisymbols              *  1.2.0       1.2.0
5   Imports     crayon              *  1.3.4       1.3.4
6   Imports       curl         >= 2.7    3.2         3.2
7   Imports       desc              *  1.2.0       1.2.0
8   Imports         fs              *  1.2.2       1.2.2
9   Imports         gh              *  1.0.1       1.0.1
10  Imports      git2r >= 0.21.0.9002 0.21.0 0.21.0.9002
11  Imports       glue  >= 1.2.0.9000  1.2.0  1.2.0.9000
12  Imports   rematch2              *  2.0.1       2.0.1
13  Imports      rlang              *  0.2.1  0.2.0.9001
14  Imports  rprojroot         >= 1.2  1.3-2       1.3.2
15  Imports rstudioapi              *    0.7  0.7.0.9000
17  Imports    whisker              *  0.3-2         0.4
18 Suggests       covr              *  3.1.0       3.0.1
19 Suggests      knitr              *   1.20        1.20
20 Suggests     magick              *    1.9         1.6
21 Suggests  rmarkdown              *    1.9       1.9.8
22 Suggests   roxygen2              *  6.0.1  6.0.1.9000
23 Suggests   spelling              *    1.1         1.1
24 Suggests     styler              *  1.0.1    1.0.9000
25 Suggests   testthat       >= 2.0.0  2.0.0       2.0.0
26 Suggests      withr              *  2.1.2       2.1.2

@jennybc
Copy link
Member

@jennybc jennybc commented Jun 1, 2018

The minimum versions stated (or unstated) are all quite intentional. It feels weird to set them all en masse, but perhaps that just means use_tidy_versions() is not relevant to usethis itself at this point in its development. It feels unsustainable or overkill to set minimum versions for everything and to default to current CRAN version. But I will set this aside and accept that this is the purpose of use_tidy_versions(). Should it be renamed use_cran_versions()?

Is this how it should work?

DESCRIPTION version is * or lower than CRAN ➡️ replace with >= <CRAN version>

Otherwise, leave DESCRIPTION version unchanged.

If DESCRIPTION version is ahead of CRAN and package is not also listed in Remotes, message about that. Although, it seems like Travis/AppVeyor will inform you of this sort of omission very quickly anyway and this function itself will no longer create this problem.

Is this just about Imports or is it for everything addressed by desc::desc_get_deps()?

It seems there is no need to even consult the locally installed version any more. This is entirely about DESCRIPTION and what's available from CRAN, yes?

@hadley
Copy link
Member

@hadley hadley commented Jun 1, 2018

Ooooh, I'm starting to remember why it uses your installed versions now — in an ideal world, you would set the required version to the lowest version of the package that your package works with. We don't have any systematic way to do that so setting to the package versions you currently have installed is a reasonable starting place.

So maybe I'm confused and use_tidy_versions() should not set CRAN versions (although for the tidyverse package, where you do want to force an update, it would be nice to have that as an option).

@jennybc
Copy link
Member

@jennybc jennybc commented Jun 1, 2018

@romainfrancois What motivates you to use use_tidy_versions()?

Maybe all this function should do is return a tibble sort of like mine above, possibly with a variable added for remarks. I can't really think of a clearly helpful operation on DESCRIPTION, but getting all the information summarized nicely for inspection is very helpful.

@romainfrancois
Copy link
Contributor Author

@romainfrancois romainfrancois commented Jun 1, 2018

I’m not sure, but I think this was about making sure travis would use the same version as locally.

« Use the same version as me, and if they are not on cran, tell me so that i can edit the remotes »

@jennybc
Copy link
Member

@jennybc jennybc commented Jun 1, 2018

@hadley and I discussed in Slack. I think use_tidy_versions() is a good candidate for conversion to a report_() function. It could just reveal the tibble I have above, possibly with a variable of "remarks", so it's easier for the developer to make informed decisions. I don't think it's clear we can make good decisions automatically in this case.

There are a couple of other awkward use_*() functions that might get a similar makeover. For example, a function to report on current dependencies and the implications of adding/deleting a dependency.

@jennybc
Copy link
Member

@jennybc jennybc commented Aug 9, 2018

Removed from the milestone, revisit for next release, when devtools scene is even more settled.

@hadley
Copy link
Member

@hadley hadley commented Nov 30, 2018

I implemented the option to use CRAN versions because I'm on a usethis issue closing rampage, but generally, I'm not sure when we should be setting package versions except in packages like tidyverse where we want to always force an upgrade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants