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

[DOC] add examples/clarify CONTRIBUTORS.md #1566

Merged
merged 4 commits into from
Apr 11, 2019

Conversation

jdkent
Copy link
Collaborator

@jdkent jdkent commented Apr 1, 2019

Changes proposed in this pull request

fixes #1565

  • add example pull requests to each type (e.g. STY, DOC, ENH, etc)
  • add link to add/commit/push files
  • change pull request link to forked repositories (since that is the workflow that fmriprep promotes)
  • add a cloning step (so contributors have explicit instructions on how to clone the repository)

Documentation that should be reviewed

The CONTRIBUTORS.md file

@ariveradompenciel could you see if this updated CONTRIBUTING.md addresses your suggestions?

@effigies, @oesteban, @emdupre : I do not know what RTM means (it's not Read The Manual), sorry I'm not keeping up with the lingo. But I would like to add an entry for that, which I can do as soon as I know what it means.

@emdupre
Copy link
Collaborator

emdupre commented Apr 1, 2019 via email

Copy link
Collaborator Author

@jdkent jdkent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple changes to make sure the article isn't only the English version (I'm assuming)

CONTRIBUTING.md Outdated
@@ -229,11 +231,21 @@ You're awesome. :wave::smiley:
[link_helpwanted]: https://github.com/poldracklab/fmriprep/labels/help%20wanted
[link_feature]: https://github.com/poldracklab/fmriprep/labels/feature

[link_pullrequest]: https://help.github.com/articles/creating-a-pull-request/
[link_pullrequest]:https://help.github.com/en/articles/creating-a-pull-request-from-a-fork
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot a space here and I should get rid of the /en/ in the path name

CONTRIBUTING.md Outdated
[link_fork]: https://help.github.com/articles/fork-a-repo/
[link_pushpullblog]: https://www.igvita.com/2011/12/19/dont-push-your-pull-requests/
[link_branches]: https://help.github.com/articles/creating-and-deleting-branches-within-your-repository/
[link_add_commit_push]: https://help.github.com/en/articles/adding-a-file-to-a-repository-using-the-command-line
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove /en/ here too.

@effigies
Copy link
Member

effigies commented Apr 2, 2019

Yeah, it would be ready-to-merge, but I'm not sure we ever actually use it. More often, we tag with WIP and just remove the tag when it's ready for review/merge.

@jdkent
Copy link
Collaborator Author

jdkent commented Apr 2, 2019

So it's kind of complimentary with WIP, where a "currently being worked on" pull request would be labelled with [WIP], and then when it's ready to review/merge, it would be marked with [RTM]. (except contributors typically do not need to be aware of this semantics or use these symbols). If I understood correctly, then I think I got it. Thanks!

@ariveradompenciel
Copy link
Contributor

Thank you! This definitely addresses my suggestions!

@effigies effigies merged commit d29902e into nipreps:master Apr 11, 2019
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.

Suggestions for CONTRIBUTING.md
4 participants