-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: Document code contributors on website #12774
Conversation
looks nice !
… Message ID: ***@***.***>
|
Okay @drammock I think I'm ready for review here! What I came up with was:
For ease of reviewing, I zipped the json files to a single zip, as I assumed that without doing so the |
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.
did interactive review with @larsoner in real-time, hence no in-line suggestions. Approving on assumption that he took good notes and will fix everything we discussed.
These are explicitly NOT in scope for this PR (but can be in follow-up PRs):
- tweaks to visual presentation
- changes to which modules we show credit for
- adding additional metrics (such as PR reviews) --- again, we want to do this, and can/should do so in follow-up PRs.
- performance enhancements relating to the GH API queries / JSON parsing (I'm convinced that it's fast enough for now)
cf4baaf
to
24fb0d4
Compare
Also based on sphinx-doc/sphinx#8791 I'm adding Will merge assuming everything works! |
728542a
to
f931f0f
Compare
Okay changed to something with more contrast that looks okay in grayscale, will merge once I manually check by eye it rendered correctly! |
@@ -1655,6 +1657,7 @@ def setup(app): | |||
"""Set up the Sphinx app.""" | |||
app.connect("autodoc-process-docstring", append_attr_meth_examples) | |||
# High prio, will happen before SG | |||
app.connect("builder-inited", generate_credit_rst, priority=10) |
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.
FYI @drammock I ended up changing the structure a bit. I realized that having a credit.rst
in our code history that gets updated with thousands of badges is less than ideal -- it'll change a lot and bloat things I think. So instead now I have it create a doc/code_credit.inc
on the fly (it's very fast) at the start of the doc build so that we don't have to keep it in our history. In theory this could instead be a directive, but I think it's actually nicer as a .inc
because you can inspect it after it's generated, don't have to deal with docutils and nested parsing at all, etc.
This gets us started on keeping track of code contributions to MNE, the first part of #11605.
Create a "Contributors" page, and link to it from the main landing page
Add a "Code Credit" heading on that page, mentioning eventually we'll add other stuff.
Populate it with code credit based on lines changed in commits in MNE-Python:
We have a lot of contributors and a lot of modules, so it's tough to get all the information in there. There are lots of potential ways to do this, what to include, how to show it -- and thus a lot of potential bike shedding, so we should probably just try to make sure what's here is an improvement over the author lists we currently use. Let's not shoot for perfection here but just "good enough"!
Still some stuff to do:
gh
command-line tool, but querying every merged PR and get files changed and lines changed in each file takes forever. Getting the merge commit -- so that we could then use thegit history
to figure things out -- sometimes returns commit hashes that don't exist in our codebase. So it's not trivial.)prs.zip
mne/
files in separate PR