Skip to content
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

[MRG+1] DOC Add libraries.io and changelog links #11298

Merged
merged 3 commits into from
Jun 18, 2018

Conversation

twosigmajab
Copy link
Contributor

The first commit in this PR closes #11287 by adding a "subscribe to releases on libraries.io" link to the "what's new" page:
screen shot 2018-06-16 at 12 19 12

I also noticed that there is no link to the changelog/what's new page from the https://github.com/scikit-learn/scikit-learn#readme content on GitHub, but I thought there should be: The "what's new" content is often the one thing that upgrading users are looking for, so I think it should be linked to from every top-level page. So the second commit in this PR adds a changelog link to the README that is displayed on GitHub, as well as another "subscribe to releases" link there as well.

Copy link
Member

@qinhanmin2014 qinhanmin2014 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only making the suggestion in one place seems enough from my side. This will be hard to maintain.

README.rst Outdated
See the `changelog <http://scikit-learn.org/dev/whats_new.html>`__
for a history of notable changes to scikit-learn.

.. The following two lines are duplicated in doc/whats_new.rst
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think comments like this in a README are appropriate. I'd rather remove this and the following reference to libraries.io. It is sufficient to have in what's new, or perhaps in installation instructions

@twosigmajab
Copy link
Contributor Author

I agree it's not ideal to have those comments to compensate for GitHub's impoverished rst renderer but think the benefit to users of also having the libraries.io link in the README far outweighed the small maintenance cost of having the link duplicated in both places.

That said, I have removed the libraries.io link from the README and the associated comments so that this can be merged assuming this PR is now 👍.

Copy link
Member

@qinhanmin2014 qinhanmin2014 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @twosigmajab

@@ -78,6 +78,12 @@ or ``conda``::
The documentation includes more detailed `installation instructions <http://scikit-learn.org/stable/install.html>`_.


Changelog
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm fine with this unrelated change

@qinhanmin2014 qinhanmin2014 changed the title add libraries.io and changelog links [MRG+1] DOC Add libraries.io and changelog links Jun 18, 2018
@jnothman jnothman merged commit 83d9a40 into scikit-learn:master Jun 18, 2018
georgipeev pushed a commit to georgipeev/scikit-learn that referenced this pull request Jun 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

include "subscribe to releases on libraries.io" in docs
4 participants