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

Correct https git URIs in documentation #9648

Merged
merged 1 commit into from Nov 1, 2017

Conversation

Projects
None yet
5 participants
@mastbaum
Copy link
Contributor

commented Nov 1, 2017

Fix a few instances in the documentation where http(s):// git URIs contained a : rather than a / to separate the host from the path.

Using a colon is malformed, and results in an error (at least on my git 2.13.3, perhaps other versions try harder to guess):

$ git clone https://github.com:matplotlib/matplotlib.git
Cloning into 'matplotlib'...
fatal: unable to access 'https://github.com:matplotlib/matplotlib.git/':
Port number ended with 'm'

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way
fix typos in https uris in documentation
Fix a few instances in the documentation where http(s):// git
URIs contained a colon rather than a slash to separate the host
from the path.

Using a colon is malformed, and results in an error (at least on
my git 2.13.3, perhaps other versions try harder to guess):

```
$ git clone https://github.com:matplotlib/matplotlib.git
Cloning into 'matplotlib'...
fatal: unable to access 'https://github.com:matplotlib/matplotlib.git/':
Port number ended with 'm'
```
@anntzer

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2017

No, the form with a colon is correct, you should not specify https but (literally)

git clone github.com:matplotlib/matplotlib.git

Conversely the form with a slash needs explicitly https:// -- we just need to settle on one. However I would strongly suggest opening the issue at https://github.com/matthew-brett/gitwash instead, which is where these files get generated from.

There is indeed a typo in gith hub that needs to be fixed.

@QuLogic

This comment has been minimized.

Copy link
Member

commented Nov 1, 2017

It's already written with http:// though, so the colon is incorrect. github.com:matplotlib/matplotlib.git is also wrong in that it doesn't specify the user name. Since that would require setting up SSH keys, I don't think we want to prescribe that and https is the way to go.

However, I do agree that this should be sent to gitwash as well.

@anntzer

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2017

oh is it just for me or does github render https://github.com/matplotlib/matplotlib/pull/9648/files stripping the 'https://'? (looking at https://github.com/matplotlib/matplotlib/commit/4bcf44940644979a06a2c8a220d95668590a1fd2.patch the patch is clearly correct indeed).

@QuLogic

QuLogic approved these changes Nov 1, 2017

@tacaswell tacaswell added this to the v2.1.0-doc milestone Nov 1, 2017

@dstansby
Copy link
Contributor

left a comment

👍 thanks a lot!

@dstansby dstansby merged commit 663e30f into matplotlib:master Nov 1, 2017

8 checks passed

ci/circleci: docs-python27 Your tests passed on CircleCI!
Details
ci/circleci: docs-python35 Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 7f8e63a...4bcf449
Details
codecov/project/library 61.65% (target 50%)
Details
codecov/project/tests 98.76% remains the same compared to 7f8e63a
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
lgtm analysis: Python No alert changes
Details

meeseeksdev bot pushed a commit that referenced this pull request Nov 1, 2017

QuLogic added a commit that referenced this pull request Nov 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.