Skip to content

Vignette - Sharing common code across analyses #142

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

Merged
merged 1 commit into from
Feb 1, 2019
Merged

Vignette - Sharing common code across analyses #142

merged 1 commit into from
Feb 1, 2019

Conversation

timtrice
Copy link
Contributor

John, apologies for not getting this completed.

@jdblischak
Copy link
Member

Thanks for getting this started @timtrice! I'll polish it and then it'd be great if you could review my changes.

For future reference, this PR is the result of the discussion in Issue #111

@jdblischak jdblischak merged commit 4390c8e into workflowr:dev Feb 1, 2019
@jdblischak
Copy link
Member

@timtrice Also note that I rebased and then merged. This keeps the commit history easy to follow, but it does change the SHA1 of your commit. The consequence of this is that it will be a bit more difficult for you to update your dev branch.

There are various ways to deal with this. One way is to discard your commit and then pull from my repo:

cd workflowr
git checkout dev
# Erase your most recent commit
git reset --hard HEAD~1
# Add my repo as a remote
# If you use SSH
git remote add upstream git@github.com:jdblischak/workflowr.git
# alternate if you use HTTPS
git remote add upstream https://github.com/jdblischak/workflowr.git
# Pull from my repo to pull in your commit plus all the other recent changes
git pull upstream dev

Please let me know if you have any questions about this.

@jdblischak
Copy link
Member

@timtrice I've edited the vignette. Could you please review/approve my changes?

https://github.com/jdblischak/workflowr/blob/dev/vignettes/wflow-07-common-code.Rmd

Also, I added you as a contributor in 30f2e03.

@jdblischak
Copy link
Member

@timtrice Just a reminder. Could please you give the vignette a quick proofread to make sure there aren't any obvious mistakes?

@timtrice
Copy link
Contributor Author

timtrice commented Feb 9, 2019

John, this looks good. But it seems I can generate plots now without changing the knit_root_dir setting. Is this expected or have I forgotten something?

@jdblischak
Copy link
Member

@timtrice Thanks for reviewing the vignette!

But it seems I can generate plots now without changing the knit_root_dir setting. Is this expected or have I forgotten something?

That is unexpected. I just tested it again and still observe the behavior described in the vignette. To summarize:

  • Use child document via chunk option -> no figures
  • Use child document via knit_expand() and knit_root_dir: "." -> no figures
  • Use child document via knit_expand() and knit_root_dir: "analysis" -> figures

@timtrice
Copy link
Contributor Author

John, I wasn't able to replicate it in rstudio.cloud so I clearly did something wrong. I think everything is set now.

@timtrice timtrice deleted the dev branch February 12, 2019 15:48
@jdblischak
Copy link
Member

@timtrice Great! Thanks for your help!

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

Successfully merging this pull request may close these issues.

2 participants