search

AbdnCHDS / guidebook

Our guide for open and reproducible ways of working in ACHDS
Other
3 stars 5 forks source link

Added more details to contributing guidelines #22

Closed dblana closed 3 years ago

dblana commented 3 years ago

Closes #19

dblana commented 3 years ago

I've just edited the readme file, please let me know if the contributing guidelines are clear! Do you think it's ok that the contributing guidelines are part of the readme file, or should they be a separate file? (Not sure what we would put in the readme file then...?)

Also @ClaraGlocke: the recommendation is to have a different branch for each "issue" you are working on (and link them via the pull request - see step 6). So if you look at the list of issues, you can see that the "update contributor guidelines" issue is being worked on by me, in the dblana-guidelines branch (you can see the branch if you click on the associated pull request). Is there a reason why you are using one branch for everything you are doing? I can't tell what you are working on that way 🤔

ClaraGlocke commented 3 years ago

@dblana Hello Dimitra,

Thanks for sending the reviews. I will look at these later today hopefully.

The answer to the branch query lies in my confusion and was solved by our discussion last week, through which I learnt that you open a branch for each section you want to work on, (even though this repeated the whole project in the branch, not just the section you want to work on, and this is what had caused the confusion). So I will be creating one branch for one issue from now on. Sorry for any muddle caused.

dblana commented 3 years ago

Oh, no muddle caused at all! The one-branch-per-issue is the recommended github workflow, but I thought maybe there was a good reason you had discovered for keeping the same branch... (I didn't even know you can keep working on a branch after you have merged a pull request - you're a pioneer 😄 )

ClaraGlocke commented 3 years ago

I think a lot of it is caused because I am so new to Github and learning as I go along. There is a chapter in the R book on Github so hopefully that will help. :-)

ClaraGlocke commented 3 years ago

@dblana Hello again!

I am putting my comments here rather than changing the actual file, as I have a few changes, which would mean unlot of work for you to undo them if you don't think they are the best option. If you want to implement some of the suggestions I am happy to add them myself after discussion or leave you to make the changes. Whichever you prefer.

The guidelines are great and an essential piece of kit. I think you have done a lovely and clear job. My main suggestions are minor and on the order the information is presented. Here are my thoughts:

  1. To reflect the workflow process, move section 4 and make it section 1 and make section 5 the new section two. (Current §1,2 become 3 & 4)
  2. If we are using the Guidebook as a learning tool for Github, could we spell out the whole 1 branch/1 issue principle?
  3. Along the same lines, perhaps add a section on reviewing and how to do this? - it can be confusing with all the mulitple buttons!
  4. Put in a paragraph as to who does the merging, otherwise people might get confused as to whether its the reviewer or the author who merges. (Does Github collect stats on the merging? Is this why the author does the merging?)
  5. Remove the 'Participation Guidelines' header and place the paragraph underneath just before the 'Contributor Guidelines' header.
  6. Swap the order of §6 & §7.

I think it is helpful to put the guidelines in the Readme file. Admitedly this is not where they usually would be, as the Readme file would be full of technical details, but we are not adding data and code and in a way the guidelines form and describe our methodology.

Thank you for all this work! :sparkles: :sparkles:

dblana commented 3 years ago

Hi @ClaraGlocke, while I'm waiting for the rendered book to update (I toggled some settings on Github and it may take some time), I made some changes if you'd like to have another look! I didn't add more info on reviewing, because the link is quite detailed (with pictures!) so I wasn't sure what more to add.