use_version function #223

EmilHvitfeldt · 2018-01-21T05:33:04Z

Fixes #188

I would like to resolve #188 and I think I have a solution ready here.

It defaults to an interactive menu where the current version is shown and you are asked which level and how much you would like to increment with. Furthermore does it have arguments for faster non-interactive use. :)

jennybc · 2018-02-15T17:27:08Z

@hadley Will you indicate your opinion at a high level? If you basically support the goal, I can shepherd the PR further along.

hadley · 2018-02-15T20:44:25Z

Seems like a little too much interactivity right now - I think if you want to increment in some non-standard way, you'll need to use the arguments. Also need to adjust use_dev_version() to use the same underlying code.

Overall seems like a good idea, although the implementation needs some work.

jennybc · 2018-02-15T21:05:07Z

@EmilHvitfeldt Let me know if you want more concrete advice before proceeding. Otherwise, just ping me when things have advanced.

I came across this function today, which you should probably look into: desc::desc_bump_version(). usethis already depends on desc, so use it freely.

EmilHvitfeldt · 2018-02-16T22:07:48Z

Thanks @jennybc! Yes I would like some more concrete advice.

So right now I feel the direction is:

A function use_version with 1 layer of interactivity with Mayor, Minor, Patch or Development. With arguments available for nonstandard increment and interactivity-avoidance.
Adjustment to use_dev_version(), possible wrapper around use_version with level = Development.

I will look at desc::desc_bump_version() Again. I remember looking at it earlier and deciding against using it.

What convention are using with regard to bumping of .9000. What increment levels should remove .9000 or should it be a separate action?

jennybc · 2018-02-17T00:53:28Z

A few thoughts:

I would hard-wire an increment of 1. That eliminates one interactive query. We just want to cover the most common development tasks.
Consider naming the argument bump, i.e. use_version(bump = "minor").
Valid values for "bump" would be c("dev", "patch", "minor", "major").
I'd present choices approximately like so

Current version is 1.2.0.9000
What part to increment?

1: Major --> 2.0.0
2: Minor --> 1.3.0
3: Patch --> 1.2.1
4: Dev --> 1.2.0.9001

or

Current version is 1.0.0
What part to increment?

1: Major --> 2.0.0
2: Minor --> 1.1.0
3: Patch --> 1.0.1
4: Dev --> 1.0.0.9000

What convention are using with regard to bumping of .9000. What increment levels should remove .9000 or should it be a separate action?

It seems like, if you have a dev version number, bumping the patch version should take you out of a dev version number.

Does this help? At this point, you've probably thought about this more than I have.

Added: I just read the docs on desc_bump_version(). Maybe you should use "which" instead of "bump" to be consistent. It feels like use_version() should be a wrapper around that function. This is appealing from a maintenance point of view.

EmilHvitfeldt · 2018-02-17T23:15:39Z

Okay @jennybc, I think this is it. I have

hardcoded the increment to 1.
removed the second layer of interactivity.
presented the resulting version numbers in the first layer of interactivity on the following format

Current version is 2.0.0
what part to increment?

1: major --> 3.0.0
2: minor --> 2.1.0
3: patch --> 2.0.1
4: dev --> 2.0.0.9000

with lowercase words to match the arguments to lessen confusing.

changed the argument name to "which".

I tried using the desc_bump_version() function but I could not avoid printing method while keeping the version update. To alleviate that I included a function bump_version in the helpers.R that did the same underlying task.

jennybc · 2018-02-19T16:34:17Z

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: usethis
 Title: Automate Package and Project Setup
-Version: 1.2.0.9000
+Version: 1.1.1.9000


Please remove this change.

jennybc · 2018-02-19T16:45:31Z

I tried using the desc_bump_version() function but I could not avoid printing method while keeping the version update

I don't quite understand this and would like to. Do you mean it's impossible to just get a version string back from the desc package vs. actually changing the version? I think I'm beginning to see the problem....

jennybc · 2018-02-19T16:54:53Z

R/helpers.R

@@ -237,3 +237,28 @@ view_url <- function(..., open = interactive()) {
  }
  invisible(url)
 }
+
+bump_version <- function(ver, choice) {


I'd stick with same vocabulary and input values. So "which" instead of "choice" and c("major", "minor", "patch", "dev") as possible values.

jennybc · 2018-02-19T16:56:41Z

tests/testthat/test-use-version.R

@@ -0,0 +1,97 @@
+context("use_version.R")
+
+test_that("use_version() increments Mayor by 1 and resets other fields", {


jennybc · 2018-02-19T16:57:20Z

tests/testthat/test-use-version.R

+  )
+})
+
+test_that("use_version() increments patch by 1 leaving other fields alone", {


Is this a duplicate of the test above?

jennybc · 2018-02-19T17:00:21Z

I'd shift the all the case-by-case testing to be testing the bump_version() helper. Then it can be much more succinct, i.e. multiple expectations inside one test, and won't need a skip. Then you really only need one test of use_version() to make sure it's wired up correctly.

jennybc · 2018-02-19T17:01:56Z

R/version.R

+            bump_version(ver, 3),
+            bump_version(ver, 4))
+
+  if(is.null(which)) {


Feels like there should be an early check for which = NULL in a non interactive session that exits early and does nothing.

EmilHvitfeldt · 2018-02-19T17:36:13Z

I don't quite understand this and would like to. Do you mean it's impossible to just get a version string back from the desc package vs. actually changing the version? I think I'm beginning to see the problem....

Yes the desc package will calculate the updated version string and update the DESCRIPTION at the same time.

jennybc · 2018-02-19T21:18:29Z

Thanks to @gaborcsardi:

There is a way to use desc's version incrementation logic w/o changing the version of the active package:

r-lib/desc#61 (comment)

Are you game to put this inside bump_version()? I would like to NOT maintain a separate version of this logic but, instead, get it from desc.

EmilHvitfeldt · 2018-02-19T21:43:41Z

That is amazing! I put it inside bump_version().

jennybc · 2018-02-19T22:29:24Z

Thanks! Will you add a bullet to the dev version section of NEWS? I might groom this a bit after the merge but will cc you on those commits, so you're in the loop.

EmilHvitfeldt · 2018-02-20T01:55:55Z

I'm a little unsure of how I would change the NEWS.md file since my forked directory is behind and doesn't have the up to date NEWS.md.

jennybc · 2018-02-20T02:35:26Z

I'm a little unsure of how I would change the NEWS.md file since my forked directory is behind and doesn't have the up to date NEWS.md.

OK I'll take it from here then.

Next time, you should create a branch in your fork, then do your pull request work in that branch. If master changes during the process, you would pull those changes into master in your fork, then merge master into your branch.

But this PR is fine. We are developing a guide for this, but it's not in position yet.

Thanks!

use_version function

b968b19

jennybc requested a review from hadley February 15, 2018 17:27

EmilHvitfeldt added 3 commits February 17, 2018 12:08

streamlining code

e75aaaf

use_version documentation

1d42a08

finalizing code without desc_bump_version

555cb69

jennybc reviewed Feb 19, 2018

View reviewed changes

EmilHvitfeldt added 2 commits February 19, 2018 10:06

input consistency and spelling mistakes

c784bf5

testing bump_version case-by-case

efa7500

using desc to update version. r-lib/desc#61 (comment)

e9637a3

jennybc merged commit 617e24f into r-lib:master Feb 20, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use_version function #223

use_version function #223

EmilHvitfeldt commented Jan 21, 2018 •

edited by jennybc

jennybc commented Feb 15, 2018

hadley commented Feb 15, 2018

jennybc commented Feb 15, 2018 •

edited

EmilHvitfeldt commented Feb 16, 2018

jennybc commented Feb 17, 2018 •

edited

EmilHvitfeldt commented Feb 17, 2018 •

edited

jennybc Feb 19, 2018

jennybc commented Feb 19, 2018

jennybc Feb 19, 2018

jennybc Feb 19, 2018

jennybc Feb 19, 2018

jennybc commented Feb 19, 2018

jennybc Feb 19, 2018 •

edited

EmilHvitfeldt commented Feb 19, 2018

jennybc commented Feb 19, 2018

EmilHvitfeldt commented Feb 19, 2018

jennybc commented Feb 19, 2018

EmilHvitfeldt commented Feb 20, 2018

jennybc commented Feb 20, 2018

		@@ -0,0 +1,97 @@
		context("use_version.R")

		test_that("use_version() increments Mayor by 1 and resets other fields", {

use_version function #223

use_version function #223

Conversation

EmilHvitfeldt commented Jan 21, 2018 • edited by jennybc

jennybc commented Feb 15, 2018

hadley commented Feb 15, 2018

jennybc commented Feb 15, 2018 • edited

EmilHvitfeldt commented Feb 16, 2018

jennybc commented Feb 17, 2018 • edited

EmilHvitfeldt commented Feb 17, 2018 • edited

jennybc Feb 19, 2018

Choose a reason for hiding this comment

jennybc commented Feb 19, 2018

jennybc Feb 19, 2018

Choose a reason for hiding this comment

jennybc Feb 19, 2018

Choose a reason for hiding this comment

jennybc Feb 19, 2018

Choose a reason for hiding this comment

jennybc commented Feb 19, 2018

jennybc Feb 19, 2018 • edited

Choose a reason for hiding this comment

EmilHvitfeldt commented Feb 19, 2018

jennybc commented Feb 19, 2018

EmilHvitfeldt commented Feb 19, 2018

jennybc commented Feb 19, 2018

EmilHvitfeldt commented Feb 20, 2018

jennybc commented Feb 20, 2018

EmilHvitfeldt commented Jan 21, 2018 •

edited by jennybc

jennybc commented Feb 15, 2018 •

edited

jennybc commented Feb 17, 2018 •

edited

EmilHvitfeldt commented Feb 17, 2018 •

edited

jennybc Feb 19, 2018 •

edited