DOC Inconsistent between checklists in CONTRIBUTING.md and contributing.rst #9958
Comments
|
Just to state the obvious, straight copy and pasting will not work because one is markdown and the other one is reST. In my mind CONTRIBUTING.md is a subset of contributing.rst, that aims to be more concise and to the point than contributing.rst. Other core devs may disagree on that. Maybe CONTRIBUTING.md could be rewritten in a nicer way with less text and more links to the dev doc (that is scikit-learn.org/dev because it will be more up-to-date than the scikit-learn.org/stable one). For example, I am not convinced that the full PR checklist should be in CONTRIBUTING.md, same thing for things like flake8, autopep8, coverage, etc ... |
|
@lesteve Thanks a lot for the instant reply :) |
|
@jnothman Could you please share your opinion on this? Do we need to make the two checklists consistent? Thanks. |
|
I'm not a big fan of contributing.md. I suspect it is read much less often than the one rendered on the web site. I would much rather work improving the rst one to make the other redundant, or simplifying CONTRIBUTING.md, renaming it to CONTRIBUTING.rst and ideally including it in the docs directly. |
|
@jnothman Thanks for the reply :)
For myself, I read the contributing.rst website. But recently I (surprisingly) find out that ISSUE_TEMPLATE.md, PULL_REQUEST_TEMPLATE.md and part of README.rst all choose to link to CONTRIBUTING.md file instead of contributing.rst website. Anyway, this PR only focus on the two PR checklists. They are almost the same, but each side is missing something from the other side. It means that before submitting PR, no matter you read the checklist in CONTRIBUTING.md or contributing.rst, you will miss something. Seems that there are 3 alternatives: |
A link to CONTRIBUTING.md is shown by github each time you open an issue or PR (probably only in a repo you have not contributed to, I haven't tested this extensively). Here is a snapshot to give an idea: As such I would bet that new contributors are way more likely to read CONTRIBUTING.md than the developers doc. Also a lot of the bullet points of CONTRIBUTING.md are specifically for these new contributors. Also an advantage of CONTRIBUTING.md is guaranteed to be up-to-date in contrary to the developers docs. Linking to the dev doc is an option I agree. The original github announcement mentions CONTRIBUTING and CONTRIBUTING.md so I am not sure you can have a CONTRIBUTING.rst. The announcement is 5 year old so it may be worth double-checking whether CONTRIBUTING.rst is working.
I am going to give you an honest answer to this one since you asked multiple times. It's not that we don't care but more "do we care enough about this to do something about it?". Core developers have limited availability and we have to make the most of their time to move the project forward. It is super easy to spend a lot of time on bikeshedding issues like this one (no clear cut obvious solution, lots of varied opinions) with no obvious sizeable benefit. Spending time on bikeshedding issues distract us from more important issues. How do we avoid this? I don't know but I think sometimes not starting a discussion is a good option. To finish on a constructive note, here is what I would be in favour of:
On a separate note, contributing dev docs have way too many bullet points if you ask me and they could use some work to be clearer and more concise. |
|
@lesteve Thanks a lot for the detailed reply :) Closing this one since the issue seems trivial. |
|
feel free to submit PRs. they're much easier to handle than discussion in
abstract, though @lesteve's response here is great.
|
In both CONTRIBUTING.md and doc/developers/contributing.rst, we have a checklist which list the things to check before submitting a pull request. (See the list in CONTRIBUTING.md and contributing.rst). The two lists are almost the same (many entries are exactly the same). But both lists are missing something from each other. For example, in .md file, we have an entry to mention expected time and space complexity but that's not in .rst file. In .rst file, we mention PEP8 check but that's not in .md file. I think it might be better to copy-paste between the two files to make the two checklist consistent.
cc @jnothman @lesteve WDYT? Thanks :)
The text was updated successfully, but these errors were encountered: