search

NESCent / popgenInfo

Vignettes for Population Genetics in R
http://popgen.nescent.org
MIT License
21 stars 50 forks source link

update CWG2R based on suggestions by @mmlopezu #132

Closed zkamvar closed 8 years ago

zkamvar commented 8 years ago

I've added comments to let the user know to start on step 3 if they've already forked and cloned the repository.

Question: should I change the steps to:

I think it might be easier this way.

mmlopezu commented 8 years ago

:+1:

smanel commented 8 years ago

YES!

smanel commented 8 years ago

Yes...If I understand you have not made the change now as suggested above? If you make the change, you will need to change the 6 below steps?

Perhaps finally better to keep the 6 below steps:

1 Fork the popgenInfo repository 2 Clone your fork to your local machine. 3 Checkout your master branch, fetch the changes from NESCent and merge them into your fork. 4 Create a new branch and then add and commit new changes or content. 5 Push your changes to your fork. 6 Open a pull request from your fork to NESCent.

At the moment you have 5 steps in the vignette and there is finally no section 6. It is easy to create one: just before mgs<-

Finally Why not adding this as an additional step? 7-Pull request from your github fork Nescent -->and you can provide some explanations on how to do? "Once you are finished with all of your changes, you can create a pull request from your GitHub fork to NESCent"

zkamvar commented 8 years ago

@smanel,

Yes...If I understand you have not made the change now as suggested above?

if you look at the commits, I have indeed made the changes as suggested above in c62b06b.

At the moment you have 5 steps in the vignette and there is finally no section 6. It is easy to create one: just before mgs<-

I saw your comment and have started to make changes in 688403e. I'm waiting on @hlapp's answer for #134 to include images.

Finally Why not adding this as an additional step? 7-Pull request from your github fork Nescent -->and you can provide some explanations on how to do? "Once you are finished with all of your changes, you can create a pull request from your GitHub fork to NESCent"

This is actually redundant with Step 6.

The final order is:

i. Fork ii. Clone

  1. Update master
  2. Checkout new branch from master
  3. Add, commit, and push
  4. Make a pull request

The last one is a bit difficult to write as github has no concrete guides that I can point to for this kind of workflow.

zkamvar commented 8 years ago

Note, you can see all of the changes I make by clicking here: https://github.com/NESCent/popgenInfo/pull/132/files

smanel commented 8 years ago

Sorry! I was looking the bad file.

Concerning the last section (I think that this is what I need to do for my vignette), I suggest to develop

"Once your pull request passes peer review and is published, you can update your +fork by starting from Step 1: checkout your master branch, fetch the changes +from NESCent, merge those changes, REVISED YOUR VIGNETTE, and push (STEP 3) them up to the master branch on your +fork to make it even with NESCent." FINALLY CREATE THE PULL REQUEST (STEP 4)

Is it correct? If yes I will try to apply it .

zkamvar commented 8 years ago

"Once your pull request passes peer review and is published, you can update your +fork by starting from Step 1: checkout your master branch, fetch the changes +from NESCent, merge those changes, REVISED YOUR VIGNETTE, and push (STEP 3) them up to the master branch on your +fork to make it even with NESCent." FINALLY CREATE THE PULL REQUEST (STEP 4)

Is it correct? If yes I will try to apply it .

Hi @smanel,

In your case, you should continue to add and commit changes to your vignette and push those changes since your pull request (#121) is still open and has not been published.

I now realize where the confusion lies. You are currently in step 4, but there are no instructions on what to do if changes need to be made (which is to continue adding content, committing, and pushing). I will add these instructions today.

zkamvar commented 8 years ago

@smanel, in other words, no, you do not need to update your branch at this time.

smanel commented 8 years ago

I use the following R script. Now what is missing?

library(git2r) repo <- repository() checkout(repo, "26_01_16_Signal_Selection") repo status(repo) add(repo, "*") # adding all new or changed files status(repo) msg <- " review signal selection vignette: correction boxplot" commit(repo, msg, session = TRUE) status(repo) # should return nothing repo summary(commits(repo)[[1]]) push(repo, credentials = cred_token())

zkamvar commented 8 years ago

I use the following R script. Now what is missing?

library(git2r)
repo <- repository()
checkout(repo, "26_01_16_Signal_Selection")
repo
status(repo)
add(repo, "*") # adding all new or changed files
status(repo)
msg <- " review signal selection vignette: correction boxplot"
commit(repo, msg, session = TRUE)
status(repo) # should return nothing
repo
summary(commits(repo)[[1]])
push(repo, credentials = cred_token())

Nothing is missing! :smiley_cat:

zkamvar commented 8 years ago

@smanel, I have made changes to include instructions on peer review. Are these sufficient?

smanel commented 8 years ago

I think yes. But I probably need to read all the vignette again and not only short revised sections to have a global idea of the vignette. Is it possible?

zkamvar commented 8 years ago

You can download it here. I've rendered it on my machine and placed it in a zip file: CWG2R.zip

smanel commented 8 years ago

A long vignette!
I will test it again with a new vignette (I hope soon).

zkamvar commented 8 years ago

Yeah, that's what ends up happening with how-to guides. 😐

Sent from my iPhone

On Feb 14, 2016, at 12:41, Stéphanie Manel notifications@github.com wrote:

A long vignette!

I will test it again with a new vignette (I hope soon).

— Reply to this email directly or view it on GitHub.

zkamvar commented 8 years ago

If it helps, steps 1-4 in the vignette correspond to steps 1-4 in the figure: https://github.com/NESCent/popgenInfo/pull/143/files

zkamvar commented 8 years ago

@smanel, do you think this version is good enough to merge at the moment? I would argue that it's much improved over the version that exists now.

smanel commented 8 years ago

Where I am sure to read the last changes? In comment #153? In this case, yes I think that it can be merge.

zkamvar commented 8 years ago

@smanel, #153 is suggesting to change CONTRIBUTING.md, BUT it is a summary of the same flow that CONTRIBUTING_WITH_GIT2R.Rmd has. Given that this is the case, I'll merge it. If you find any problems, please create an issue and I will fix them.