Tests docs #118
Tests docs #118
Conversation
To solve some documentation mismatches detected during R CMD check the man pages were updated. The issue was fixed
| Version: 0.1.8.1 | ||
| Date: 2021-03-13 | ||
| Version: 0.1.8.2 | ||
| Date: 2021-04-08 |
There was a problem hiding this comment.
(I generally prefer to do this myself to mark releases, but I appreciate that you are also trying to set the release apart.)
There was a problem hiding this comment.
(It also prevents me from merging now as I have about the same at home :) Quick stash will help)
There was a problem hiding this comment.
Sorry for that. 😭
Some people like the bump, some people don't. Next time I ask.
There was a problem hiding this comment.
I am ambivalent. I like the attention to detail, and you made enough changes to mark a delta -- just how I think. The 'cannot merge' is just a stash away so no beans. The one thing I really dislike is using .9000 :) as there is zero sense in more than one new digit, incremented starting from zero. And while I said so for years it won't change you-know-where...
It is a bit more important in larger project / possibly use of more PRs like, say, Rcpp. This was fine. Thanks again.
There was a problem hiding this comment.
And the one missing piece (a ChangeLog entry) I now added.
There was a problem hiding this comment.
Yeah, I know. Justification is also a part of it, e.g. Why start at .9000 and not .0000? And why the long number anyway? I feel it some weird calcification from a long lost regex pattern.
| @@ -21,4 +21,4 @@ License: GPL (>= 2) | |||
| URL: https://github.com/eddelbuettel/drat, https://dirk.eddelbuettel.com/code/drat.html | |||
| BugReports: https://github.com/eddelbuettel/drat/issues | |||
| Encoding: UTF-8 | |||
| RoxygenNote: 6.0.1 | |||
| RoxygenNote: 7.1.1 | |||
There was a problem hiding this comment.
(I use an older/simpler roxygen by default hence the old version)
(As we can see below, a good part of the diff of Rd files is entirely spurious and just line breaks and style of function signature printing)
There was a problem hiding this comment.
There was one warning for me, where the location argument was in the function, but not in the docs.
See line 26 of man/pruneRepo.rd
There was a problem hiding this comment.
Ahh. Good catch. I may have to (eventually) revert all this back just because the older / faster roxygen is my default here at home. (The new one needlessly recompiles shared libraries in packages with code which I find annoying but my issue tickets on it were all closed by the powers-that-be. Oh well. And, being an ESS user and its indentation, I am not a fan of the new look and "styling". Ah well squared. I'll hide under my rock now.)
| } | ||
| runTest(wd, "docs") |
|
I use(d) I hope we can roll this out, get some experience with it and then (in due time) a) default to |
Hi @eddelbuettel
I had a look at the code changes for the new location type. Looks all fine to me and I could quite easily expand the test to explicitly test with the new location option. (I think you made the necessary changes for the test already a few weeks ago)
One thing I would like to discuss is the name of the option
dratBranch. I think this is not the correct name anymore anddratLocationwould be better suited. However, I am not sure how the renaming process would affect users.Felix