-
-
Notifications
You must be signed in to change notification settings - Fork 25.6k
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: rephrase the role of core devs #28912
Conversation
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.
LGTM. Thanks
Co-authored-by: Jérémie du Boisberranger <[email protected]>
doc/about.rst
Outdated
roles, however a complete list of contributors can be found `on github | ||
<https://github.com/scikit-learn/scikit-learn/graphs/contributors>`__. |
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.
that page only shows top 100 people, which at this point means minimum 24 PRs merged.
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.
I rephrased slightly to be more nuanced
Co-authored-by: Adrin Jalali <[email protected]>
Co-authored-by: Adrin Jalali <[email protected]>
Co-authored-by: Adrin Jalali <[email protected]>
Co-authored-by: Adrin Jalali <[email protected]>
Co-authored-by: Jérémie du Boisberranger <[email protected]>
… all teams are core contributors
Co-authored-by: Jérémie du Boisberranger <[email protected]>
Co-authored-by: Jérémie du Boisberranger <[email protected]>
Co-authored-by: Jérémie du Boisberranger <[email protected]>
Co-authored-by: Jérémie du Boisberranger <[email protected]>
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.
LGTM
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.
Thank you, @GaelVaroquaux.
LGTM modulo a few nitpicky comments, questions, and a suggestion regarding one element of yesterday's discussion (i.e the rewording of "Authors").
@@ -22,11 +22,22 @@ Governance | |||
The decision making process and governance structure of scikit-learn is laid | |||
out in the :ref:`governance document <governance>`. | |||
|
|||
Authors | |||
------- | |||
.. _authors: |
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.
Should this tag be adapted? Shall we also credit the original academical authors of scikit-learn and its associated publication?
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.
We can adapt this tag if you think that it is important. I did not think it was, as it's purely an internal tag, and I was worried of doing my git grep wrong and thus missing some places pointing there.
I'm not in favor of crediting the original academic authors and the associated publication more than they already are: this list reflect the people involved in the first couple of years, which is a small fraction of the total road that got scikit-learn to where it is today.
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.
Actually, regarding the tags, I added it to make sure that old html links that point to the anchor "#authors" in html still worked.
I'm relearning an old lesson: each time one does a non obvious thing, it should come with a comment :)
doc/about.rst
Outdated
................ | ||
|
||
The following people are currently maintainers, in charge of | ||
consolidating scikit-learn's development and maintenance: | ||
|
||
.. include:: authors.rst |
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.
Do we also want to rename authors.rst
?
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.
Done!
I hope that I did not screw something up here. I guess that we'll know next time we update the contributor list, eg when updating the emeritus
A slight adjustment in phrasing and framing to give more visibility to all contributors, and to avoid suggesting that core devs are the only authors.
This PR is done to implement the discussion of today's meeting.