Fix issue #623 (related to Collate field, and loadall with S4 classes)

[Geoff99]

This proposed change should resolve issue #623.

The additional code proposed for load_all calls roxygen2::update_collate to make sure the Collate field in the DESCRIPTION file is up to date with any @includes in the package's R directory, before attempting to read and evaluate any of the package's code. as.package is then used to refresh the pkg structure (specifically pkg$collate) used in load_all to ensure that later steps are aware of and use the up to date Collate information..

IMPORTANT This pull request should NOT be applied until the roxygen issue r-lib/roxygen2#302 has been resolved. (At the time of writing, ie with that roxygen issue unresolved) roxygen::update_collate creates a blank Collate field in (to be precise totally removes the Collate field from) the DESCRIPTION file when there are no @includes found in the .r files. Several of the testthats for devtools have exactly that scenario - viz a manually entered Collate field (rather than one generated by roxygen2 via @includes) - and the change to their DESCRIPTION files (if test() were run on devtools itself) would permanently damage the test.

I have submitted two pull requests for roxygen2, namely pull request r-lib/roxygen2#303 (to resolve the roxygen issue), and r-lib/roxygen2#304 (to add a testthat the roxygen issue is indeed resolved satisfactorily). Please review those changes before considering this request.

I will submit a separate pull request containing a testthat issue #623 is actually resolved.

Geoff

[Geoff99]

I have just realised that I probably ought to have waited for the roxygen2 changes this fix depends upon before submitting this pull request, since until the roxygen2 issue is resolved, making this change to devtools causes it to fail the Travis tests :-(

Apologies.

Mmmm, learning something new every day.

Geoff

[Geoff99]

Temporarily closing this pull request (sorry for all the toing and froing) while I learn more about using version numbers to ensure that this fix to devtools ensures that the fix to roxygen2 upon which it depends is available.

Update As at 22 Nov 2014 I think I have figured out the answers to the queries below, so don't bother reading the rest of this comment. I am only leaving them in there as a record for myself of what I was confused about (in case future me forgets again). Geoff

Advice re the following topics which have me slightly puzzled would be appreciated (I'll continue to research / learn / experiment myself but advice or pointers (eg links to what I should read) would be appreciated if anyone has time to offer them).

First : I know I can update the DESCRIPTION file so that the Suggests field says it needs a particular version of roxygen2, eg

Suggests:
    testthat (>= 0.7),
    roxygen2 (>= 4.0.2.9001), 
    ...

but I'm unclear about exactly when that gets checked (and very unclear about how it interacts with Travis, which is on the far horizon of my personal knowledge space at the moment (I know it's there but ...))

Second : I could insert code into the fix in load_all which uses packageVersion("roxygen2") >= "4.0.2.9001" to test that roxygen2 version number is sufficiently large when load_all is executed

(or I could employ the is_installed function from devtools utils.r, once I work out why it checks for > rather than >= --- it looks like I would need to code it as is_installed("roxygen2", version = "4.0.2.9000") to get the effect I want),

but I'm not sure exactly what action I should take if the roxygen version is not high enough.

• I might put out an advisory message and continue without calling update_collate (in which case users with @includes will not get what they expect, but will at least be warned); or
• I could stop , but that seems to me to imply that roxygen2 should be promoted from Suggests to Imports (but I'm still reading up on, and slowly getting my mind around the difference between Suggests, Imports and Depends, so I could be way off track here). And anyway, that seems a big change to propose for what is essentially a 1 line fix to the code !

Geoff

[Geoff99]

Hi @hadley

I've decided to leave this fix for issue #623 parked and wait until there has been time to review the pull request for roxygen2 which makes update_collate genuinely do nothing if there are no @includes in the package code (see r-lib/roxygen2#303 for the proposed fix, or r-lib/roxygen2#304 for the fix plus a testthat)

I did try (on a separate experimental branch in my forked copy of devtools) to use version numbers to make the requirement for the roxygen2 correction be 'automatically (or automagically) applied when devtools is loaded (and when Travis CI runs). I got part of the way there (and learnt lots more about Travis, and also about exactly which version numbers gets checked by what commands, and when) but it's getting too complicated (Travis CI needs devtools for some of the scripts to work) for a fix which amounts to only three lines of code (plus some testthats) so I'll just do them in series, and worry about getting the roxygen2 fix right and accepted first.

Geoff