-
Notifications
You must be signed in to change notification settings - Fork 156
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
[MISC] edit contributing guide #44
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not able to work on this much this evening, but thank you so much for getting us started @Park-Patrick!
I'll have a look again at the weekend on or Monday 😸
@@ -1,14 +1,95 @@ | |||
# Contributors guide | |||
|
|||
## Markdown style |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for starting this @Park-Patrick! Is this markdown style section incorporated into the file you've started? It does need to be, and it probably makes sense to add it as a section that can be linked from the table of contents.
Some explanation of what the continuous integration is doing, what linting is and why it needs to be done would be really great. ✨
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right makes sense!
also there is an markdown section but I'll add the link to the table of contents
:tada::balloon::cake: **Welcome to the BIDS Specification repository!** :cake::balloon::tada: | ||
|
||
:dizzy::hatched_chick::sunny: *We're so excited you're here and want to contribute.* :sunny::hatched_chick::dizzy: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the BIDS Specification will want all the emojis from the starter kit. Maybe have a think about how this can be formalised a little while also still being friendly!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah wasn't sure what we wanted to do about those, but maybe we can just stick to bold / italics then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think just bold and italics is a good starting point ! We can add emojis back in later if we think it's missing something :)
Oh - also - could you change the title of this pull request to something like "updating contributing guidelines" please - it isn't very clear in the list of pull requests what we're working on here! |
:tada::balloon::cake: **Welcome to the BIDS Specification repository!** :cake::balloon::tada: | ||
|
||
:dizzy::hatched_chick::sunny: *We're so excited you're here and want to contribute.* :sunny::hatched_chick::dizzy: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @Park-Patrick Thank you for writing this up! I liked that the guide felt digestible throughout and easy to understand (especially for those that it maybe their first time navigating GitHub). The example you provided is great! For the 'Making a change with a pull request' I suppose that was more for the starters kit? I think the principle is consistent, just in regards to the specification. I noticed that there were some double bracketed terms (i.e. in 1 (Comment on an existing issue...) the second paragraph of [This blog][dont-push-pull-request]) I wasn't sure if you meant them to linking elsewhere? I think with Kirstie's suggestions (of adding the linter and continuous integration) will make this a nice contributing guide! One thing that maybe considered adding is how the repo is organized (i.e. that the standard is currently in the src folder). I can help with some of this too!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch! so some of the double bracketed terms had corresponding tags in the starter kit version, but I think some of them became obsolete so i'll either re-link or remove them.
I'm not super familiar with this repo's policies for linter/CI moving forward, so it would be great to get help with that!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see! No worries!
I can add what I understand from linter/CI as well. I'll work on it from your repo and PR to that
CONTRIBUTING.md
Outdated
|
||
## Recognizing contributions | ||
|
||
BIDS follows the [all-contributors][all-contributors] specification, so we welcome and recognize all contributions from documentation to testing to code development. You can see a list of current contributors in the [BIDS specification][bids-specification]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you point to the right .md file here?
|
||
How do you know that you're a member of the BIDS community? You're here! You know that BIDS exists! You're officially a member of the community. It's THAT easy! Welcome! | ||
|
||
## Get in touch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this intended to be empty? We can remove it for now or add a section about opening issue to raise discussion topics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should add to this - I'm going to wait until @franklin-feingold's PR is merged to @Park-Patrick's branch and then I can add some text in there.
- mailing list
- neurostars
- github
right?
CONTRIBUTING.md
Outdated
This allows other members of the BIDS Specification team to confirm that you aren't overlapping with work that's currently underway and that everyone is on the same page with the goal of the work you're going to carry out. | ||
|
||
|
||
#### 2. [Fork][github-fork] the [BIDS Starter Kit repository][bids-starterkit-repo] to your profile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should be pointing to BIDS Starter Kit.
CONTRIBUTING.md
Outdated
|
||
#### 2. [Fork][github-fork] the [BIDS Starter Kit repository][bids-starterkit-repo] to your profile | ||
|
||
This is now your own unique copy of the BIDS Starter Kit. Changes here won't affect anyone else's work, so it's a safe space to explore edits to the code! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
I noticed that there is a lot of square bracket syntax that is not compatible with markdown - for example |
The square bracket convention will work with markdown if you specify the links at the end of the file (similar to RST links). So something like
will work. But I don't see those links in this PR, unless I'm just overlooking them ? |
I went through and cleaned up the double brackets, edited the sections pointing to the starter kit, added some information about linter and CI, and made sure there were no linter errors. I submitted a PR to Patrick's fork - patrick-g-h#1 |
Hi folks! Sorry I disappeared on you for so long! I'll go over to @Park-Patrick's fork and review there first. Ping me if there are any major questions I can help with ✨ |
Question for anyone who's following along: do we want to ask contributors to add links at the bottom of a file (using the square bracket notation that @emdupre described above #44 (comment)) or do you think it's cleaner to have each link at the time it's used? What I like about the square brackets is that you a) don't have to re-type the same link multiple times, and b) you build up a little "useful links" section at the end of every file. The problem though is that the link is not next to the text that you're linking in the raw markdown and so it might be rather confusing! Add a little 🎉 emoji to this comment if you want to use the square bracket format or a ❤️ emoji if you'd prefer the |
updated contributing guide
I found this PR and hoped that it would explain to how actually "build" the specification document from the |
Indeed it would be good to add those instructions to this document. I just added CI integration to build docs for all commits. You can learn what commands are required from the config file: https://github.com/bids-standard/bids-specification/blob/master/.circleci/config.yml |
Thank you for bring this up @yarikoptic! I have just added a short tutorial of how to build the specification locally using mkdocs. I submitted a PR to @Park-Patrick's repository. |
@Park-Patrick what's the ETA on this PR? If you are busy perhaps @franklin-feingold or @KirstieJane could take over? |
Added to contributing guide - mkdocs tutorial
Okay just merged @franklin-feingold 's most recent PR! Sorry, I thought it was the same one that I merged a few days ago. |
Thanks everyone! |
No description provided.