mne-tools / mne-python

MNE: Magnetoencephalography (MEG) and Electroencephalography (EEG) in Python
https://mne.tools
BSD 3-Clause "New" or "Revised" License
2.73k stars 1.33k forks source link

Update contributing guide #11609

Open britta-wstnr opened 1 year ago

britta-wstnr commented 1 year ago

In the last dev meeting, @drammock and I discussed that our Contributing Guide may need an overhaul.

Right now, the guide is long, hard to navigate if you search for something, and hard to understand/follow for new contributors.

The topics discussed (in order) within one page are right now: ways to contribute, configuring git, GNU make explanation, forking and upstream on GitHub, virtual environments, git commands, SSH with GitHub, documentation, API changes, changelog, local testing, test writing, coding style, naming conventions, docstrings, cross-referencing, code organization, testing, building documentation, MNE command-line tools, GitHub workflow.

While undoubtedly all of this is important, the info is overwhelming and switches between topics (which might make sense, again, if you are following while contributing, but it might be hard to connect the dots when reading this for the first time).

I propose to chop this up into several pages (make it feel less overwhelming, have information on one topic more bundled and use links to interconnect). Further, I propose to potentially have different versions for new contributors and advanced contributors. This could e.g. be realized by having bullet point summaries at the top (if you just need to read up on something you forgot) and then more detailed explanations further down.

TL;DR: Proposal to split the contributor guide into several pages and have sections that speak to the advanced contributor that just wants to look something up as well as sections that are easier to follow and give detailed explanation.

larsoner commented 1 year ago

Works for me. One way to think of beginner vs advanced topics/pages might be to target the beginner one toward someone who quickly wants to make a one or two line fix, and not concentrating on telling them everything but just enough that they might get it right on the first try. If there is too much text people won't read it, so I think it's better to be brief but incomplete and hit on the most important points. To me that's: using a GitHub workflow, TDD, and adding changes to the change log. FWIW this is what I have to comment on in beginner contributor reviews most of the time. The rest (like style issues) CIs help with and people often self correct in-PR.

Then at the end of the beginner doc we could write "some problem with your PR? See the advanced page!" And there in advanced we talk about flake, Makefile, html_dev-noplot, etc.

larsoner commented 1 year ago

@britta-wstnr do you volunteer to refactor? :) I've optimistically marked for 1.4 so we don't forget to think about trying, but if we have to bump the milestone because nobody has time that's fine

britta-wstnr commented 1 year ago

Thanks for your input @larsoner - sounds good to me. I would also like to move some extra info out onto extra pages (e.g. how to use git) - so people can easily find it through links, but it does not add a wall of text. Would that be against the philosophy of how MNE handles webpages?

Yes, I already discussed with @drammock that I'd be happy to give this a go - but I will only have an hour here and there to work on it in the upcoming weeks (just as a disclaimer).

Should I post a page overview here before I get started?

larsoner commented 1 year ago

I would also like to move some extra info out onto extra pages (e.g. how to use git) - so people can easily find it through links, but it does not add a wall of text. Would that be against the philosophy of how MNE handles webpages?

No I think that's fine. And even better I think would be to link to some standard tutorials for how to use git and how to set up a standard GitHub workflow. NumPy/SciPy/Pandas etc. all use the same workflows I think so I'd look to see what they link to. Or if they have docs of their own, just say "follow this but for MNE instead"...

larsoner commented 1 year ago

Should I post a page overview here before I get started?

If it's 10 minutes to copy-paste and open a PR where we can just look at the built docs instead then that's better. But if it's going to be 1h+ of work for the first version then sure we can think through here some more

drammock commented 1 year ago

I would also like to move some extra info out onto extra pages (e.g. how to use git) - so people can easily find it through links, but it does not add a wall of text. Would that be against the philosophy of how MNE handles webpages?

No I think that's fine. And even better I think would be to link to some standard tutorials for how to use git and how to set up a standard GitHub workflow. NumPy/SciPy/Pandas etc. all use the same workflows I think so I'd look to see what they link to. Or if they have docs of their own, just say "follow this but for MNE instead"...

For now I'd say keep whatever we already have on "how to use git" (though +1 to move it to a separate page). IMO pointing users to e.g. the Pandas docs is not great, and will take extra effort to make sure that what we're pointing them to is indeed equivalent to what we want (and stays that way). For the longer term, I'll bring this up as something that can be addressed at the level of the Scientific Python org (creation of a package-agnostic "how to use git to contribute to scientific python projects" tutorial) that all the projects could link to.

larsoner commented 1 year ago

For ref it looks like NumPy has a basic page that then links to advanced stuff

https://numpy.org/doc/stable/dev/index.html

britta-wstnr commented 1 year ago

I worked on an outline for the refactoring of the contributing guide. This is probably not complete yet - rather than the details, I'd like to discuss high-level structure now (the low-level content is more to give you an idea).

The big changes I propose are:

  1. split the page in many subpages
  2. have a workflow overview
  3. have a contribution checklist that can be consulted at different steps of the process and that links to the more detailed descriptions.

The difference between the workflow overview and the checklist would be that the overview explains what will happen during the contributing process, while the checklist should only list the responsibilities of the contributor before taking a certain step (i.e. that would also be useful for people who do not need an in-depth explanation).

Note: the numbering below is just to make it clear which site references which.

0. Getting started

1. Workflow overview

2. Setup: dev. environment

3. Coding conventions

4. Testing and building documentation

5. Contribution checklist

This is supposed to be a reference for anyone, from beginner to frequent contributor: a checklist to see if everything for the next step is considered, linking to more in-depth-info if a reminder is needed. —> for each of the points below, link applicable parts in 0.-4.

larsoner commented 1 year ago

Looks reasonable to me!

drammock commented 1 year ago

my main comment is about the git "satellite" page (or "orphan" page in rST speak). The main downside of such pages is that they take you out of the linear flow, which means they don't automatically get next/prev links at the bottom of the page. However: given that we hope to eventually outsource the git tutorial page (i.e., to a generic scientific-python page about git) I guess there's really no harm in having our git page be an "orphan" in the meantime.

Also wondering where the "should I even open a PR" content should go. It appears in the checklist, but that's supposed to be just a link back right? Does it go in section 0, maybe between "rules" and "how to get help"?

britta-wstnr commented 1 year ago

@drammock Mh, I can see the concern about the orphan page ... Apart from the reason of it being outsourced in the future, I was also driven by my inner conceptualization that this does not necessarily fit into the flow anyway - you have to know/learn about git and GitHub at different places in the workflow, but it still might be beneficial to have all git/GitHub resources bundled in one page. But I have no strong opinion here - happy to have it in a linear flow as well.

Regarding the "should I even open a PR" content: I loosely bundled it with "Ways to contribute" for now (that is where it's discussed right now), but I agree that it might make sense to make it a bigger/its own bullet point.

larsoner commented 1 year ago

Also relevant: https://github.com/mne-tools/mne-python/issues/9572

larsoner commented 1 year ago

Also relevant: https://github.com/mne-tools/mne-python/issues/6266

britta-wstnr commented 1 year ago

Thanks @larsoner - after my summer of travelling™️ comes to a close, I will put this back on the plate.

larsoner commented 4 months ago

Maybe for a lot of stuff we can point to

https://learn.scientific-python.org/development/

And then "just" list our local conventions?