Skip to content

Update contributing guide #11609

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

Open
britta-wstnr opened this issue Mar 29, 2023 · 16 comments
Open

Update contributing guide #11609

britta-wstnr opened this issue Mar 29, 2023 · 16 comments
Labels
Milestone

Comments

@britta-wstnr
Copy link
Member

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
Copy link
Member

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 larsoner added this to the 1.4 milestone Mar 29, 2023
@larsoner
Copy link
Member

@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
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

britta-wstnr commented Apr 12, 2023

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

  • Quickstart button: links to 5. contribution checklist
  • Types of contributions
  • EDIT: How to contribute ("should I open a PR?") - as per comment by @drammock
  • Rules (Code of Conduct etc)
  • How to get support if needed

1. Workflow overview

  • Overview of the workflow from installation to testing, with links to more in-depth explanations
  • Satellite page: git and GitHub

2. Setup: dev. environment

  • git configuration (linking satellite page from 1.)
  • virtual environment with dev. version of MNE
  • IDEs (cf. Configure ide #11615)
  • other tools to install
  • GNU make

3. Coding conventions

  • Code organization
  • Naming conventions
  • Cross-referencing
  • MNE conventions
  • API changes
  • Documentation/docstrings
  • test coverage (link to 4. Testing)
  • Deprecations
  • changelog

4. Testing and building documentation

  • writing tests
  • local testing: running the test suite
  • building the documentation
  • modifying command line tools

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.

  • before coding
    • e.g. should I even open a PR?, coding conventions
  • before opening a PR
    • e.g. testing, test coverage, DRAFT PRs
  • before pushing new changes
    • e.g. considering which CIs to skip, merging upstream/rebasing
  • before asking for merge
    • changelog, names.inc

@larsoner
Copy link
Member

Looks reasonable to me!

@drammock
Copy link
Member

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
Copy link
Member Author

@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 larsoner modified the milestones: 1.4, 1.5 Apr 28, 2023
@larsoner larsoner modified the milestones: 1.5, 1.6 Jul 31, 2023
@larsoner
Copy link
Member

larsoner commented Oct 1, 2023

Also relevant: #9572

@larsoner
Copy link
Member

larsoner commented Oct 1, 2023

Also relevant: #6266

@britta-wstnr
Copy link
Member Author

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

@larsoner larsoner modified the milestones: 1.6, 1.7 Nov 16, 2023
@larsoner larsoner modified the milestones: 1.7, 1.8 Apr 9, 2024
@larsoner
Copy link
Member

Maybe for a lot of stuff we can point to

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

And then "just" list our local conventions?

@larsoner larsoner modified the milestones: 1.8, 1.9 Jul 12, 2024
@larsoner larsoner modified the milestones: 1.9, 1.10 Dec 9, 2024
@dominikwelke
Copy link
Contributor

hi @larsoner @britta-wstnr -
slightly off-topic, but could it be that the environment setup recipe is also outdated?

at least for me (on mac os) it doesnt work out of the box:

  1. conda remove --force mne after creating from .yml doesnt work, as there is no conda package mne (but mne-base)
    removing mne-base instead and moving on seems to work, though.

  2. however, i am also struggling to locally build docs. dont know if there is perhaps something specific to my setup.
    with a resh environment created as described above

  • make html-noplot breaks with and error in numpydoc
ERROR: [numpydoc] While processing docstring for 'mne.Covariance'

Extension error (numpydoc.numpydoc)!
  • make html-pattern also doesnt execute correctly
make: *** [html-pattern] Killed: 9als/intro... [ 14%] 10_overview.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants