Skip to content

MRG, DOC: overhaul contributor guide#6254

Merged
larsoner merged 5 commits intomne-tools:masterfrom
drammock:dev-docs
May 6, 2019
Merged

MRG, DOC: overhaul contributor guide#6254
larsoner merged 5 commits intomne-tools:masterfrom
drammock:dev-docs

Conversation

@drammock
Copy link
Copy Markdown
Member

@drammock drammock commented Apr 30, 2019

Closes #6179

  • adds a new file CONTRIBUTING.rst to the repository root, that gives a short overview of how to contribute
  • overhauls doc/contributing.rst to give more detailed guidance and bring it up to date.
  • ultimately remove doc/configure_git.rst and doc/customize_git.rst; give just enough detail in doc/contributing.rst to get people off the ground, and point them to the many other resources for further git learning

cc @massich @jasmainak @jeythekey

@larsoner
Copy link
Copy Markdown
Member

This is a long-needed overhaul, thanks for tackling this @drammock

@codecov
Copy link
Copy Markdown

codecov bot commented May 1, 2019

Codecov Report

Merging #6254 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #6254   +/-   ##
=======================================
  Coverage   89.18%   89.18%           
=======================================
  Files         416      416           
  Lines       74954    74954           
  Branches    12353    12353           
=======================================
  Hits        66845    66845           
  Misses       5225     5225           
  Partials     2884     2884

@larsoner
Copy link
Copy Markdown
Member

larsoner commented May 1, 2019

If it's good to go, feel free to set to MRG and tell me you did it (GitHub does not notify of title changes) and I'll take a deeper look at the CircleCI render

@drammock drammock changed the title WIP, DOC: overhaul contributor guide MRG, DOC: overhaul contributor guide May 1, 2019
@drammock
Copy link
Copy Markdown
Member Author

drammock commented May 1, 2019

Here is the rendered page:
https://12938-1301584-gh.circle-artifacts.com/0/dev/contributing.html

@larsoner @massich ready for review.

@ikojcic you might also be interested, since we discussed the need for this last week.

@drammock drammock requested a review from massich May 1, 2019 20:17
@drammock drammock marked this pull request as ready for review May 1, 2019 20:22
@larsoner
Copy link
Copy Markdown
Member

larsoner commented May 1, 2019

We can not require master versions, this is a temporary thing / corner case hopefully. We should make our docs at least build (if not as beautifully) with latest releases of each anyway. I think all we miss is a skip in the conf.py for the escape char currently.

Copy link
Copy Markdown
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Just a few small comments to start, I'll look fully tomorrow. Feel free to fix now or wait until I comment more fully.

$ # (same command works to add a completely new file):
$ git add mne/some_file.py
$ # enter interactive staging mode, to add only portions of the file:
$ git add -p mne/viz/some_other_file.py
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This interactive bit seems like overkill / expert territory.

FWIW 99% of the time I just do git commit -am "whatever". Might be simpler than telling people to do it one by one.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When I was starting with git learning about -p actually made my life easier - beginnings are often messy and its good to know how to clean up. So it would be nice to still keep this information somwhere.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've left this in because I agree with @mmagnuski, add -p is really useful for beginners I think.

Copy link
Copy Markdown
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Okay I think that's it for me :)

@drammock drammock mentioned this pull request May 3, 2019
12 tasks
@drammock
Copy link
Copy Markdown
Member Author

drammock commented May 3, 2019

Copy link
Copy Markdown
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

besides my nitpick it's great !!!

@drammock
Copy link
Copy Markdown
Member Author

drammock commented May 6, 2019

@larsoner
Copy link
Copy Markdown
Member

larsoner commented May 6, 2019

Great, thanks @drammock ! I'll merge and if there are more tweaks people want, we can make new PRs

@larsoner larsoner merged commit 39f46ad into mne-tools:master May 6, 2019
@drammock drammock deleted the dev-docs branch May 6, 2019 18:22
@agramfort
Copy link
Copy Markdown
Member

agramfort commented May 6, 2019 via email

@massich
Copy link
Copy Markdown
Contributor

massich commented May 14, 2019

great!

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.

DOC ENH: better setup instructions for new contributors

7 participants