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

Only update changed collate #325

Merged
merged 6 commits into from Oct 5, 2015
Merged

Conversation

@Geoff99
Copy link
Contributor

@Geoff99 Geoff99 commented Feb 25, 2015

Hi @hadley

This pull request relates to devtools issue r-lib/devtools#723

@renozao neatened up my first quick fix to make update_collate rewrite the Collate entry in the DESCRIPTION file only when roxgyen2 finds changes generated by #' @includes in the package. He also wrote a unit test as well.

This code won't guarantee that devtools can run safely in parallel (that's a much bigger job I think) but I think it will resolve the use case which generated the devtools issue (or at least return control of the problem to anyone running multiple load_alls at the same time on a package)

Happy to respond to any changes you'd like made. Thanks in advance for looking over this.

@renozao
Copy link

@renozao renozao commented Feb 26, 2015

I don't think we need to tackle the parallel loading in general.

I believe that for plain R packages it should be enough to ensure that no files is changed if not necessary. The use case is that the package has first been loaded by load_all in a separate session before running the parallel jobs, so that each job, when individually loading the package with load_all should not overwrite anything.

@hadley
Copy link
Member

@hadley hadley commented Oct 4, 2015

Could you please add a bullet point to NEWS?

@Geoff99
Copy link
Contributor Author

@Geoff99 Geoff99 commented Oct 5, 2015

Bullet point about update_collate only rewriting the Collate entry after changes has been added to NEWS, as requested.

@hadley
Copy link
Member

@hadley hadley commented Oct 5, 2015

Can you please put the NEWS under the top-most bullet point? (should be under 4.1.1.9000) (and similarly for your other PR)

@Geoff99
Copy link
Contributor Author

@Geoff99 Geoff99 commented Oct 5, 2015

NEWS entries moved.

@hadley hadley merged commit 55adc09 into r-lib:master Oct 5, 2015
1 check passed
@hadley
Copy link
Member

@hadley hadley commented Oct 5, 2015

Thanks!

@Geoff99 Geoff99 deleted the only-update-changed-collate branch Oct 5, 2015
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

3 participants