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

Propose change to update_collate to fix issue #302 #303

Closed
wants to merge 4 commits into from

Conversation

@Geoff99
Copy link
Contributor

@Geoff99 Geoff99 commented Nov 16, 2014

This change to update_collate should resolve issue #302 by making update_collate genuinely do nothing if no @includes are found in the package being updated. I have also modified the documentation slightly to clarify what update_collate (now) does.

Without this change update_collate sets the Collate field in the DESCRIPTION file to blank (to be precise, removes it entirely) if no @includes are found. This effectively wipes out any Collate field values that were manually entered. This is undesirable - for example a number of the testthats in devtools use template packages where the Collate sequence matters, and where the Collate field has been entered manually.

I have also coded a testthat for this fix. I will submit that as a separate pull request.

Once / if this pull request is accepted, there is a simple fix for issue r-lib/devtools#623

Any advice and feedback most welcome

Geoff

PS Apology for the slightly misleading title on the first commit message from the chain of commits while I was working on this - I am still learning how to use git properly.

Geoff99 added 3 commits Nov 14, 2014
to reflect the fix that makes it truly do nothing, instead of creating a blank Collate field when not \code{@@includes} are found.
So it can be referenced in Imports field in DESCRIPTION file for devtools
@hadley hadley closed this in 6bf160e Dec 9, 2014
@hadley
Copy link
Member

@hadley hadley commented Dec 9, 2014

Thanks! I ended up simplifying the whole function because of this change.

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

Successfully merging this pull request may close these issues.

None yet

2 participants