Undefined Stars / Forks #329

Closed
alexcurtin opened this Issue May 15, 2017 · 8 comments

Comments

Projects
None yet
3 participants
@alexcurtin

alexcurtin commented May 15, 2017

Description

Github Stars/Forks says Undefined Stars / Undefined Forks.

Expected behavior

The github area shows stars/forks

Actual behavior

Github Stars/Forks says Undefined Stars / Undefined Forks.

Steps to reproduce the bug

  1. set repo_name: and repo_url in mkdocs.yml
  2. private repo?
  3. see bug.

Am I getting undefined stars / forks for my github url because its a private repository? If so, can we add a way to have our container auth to github before this function makes its call to the repository to find # of stars and forks?

@squidfunk

This comment has been minimized.

Show comment
Hide comment
@squidfunk

squidfunk May 16, 2017

Owner

Do you have the same error with a public repo?

Owner

squidfunk commented May 16, 2017

Do you have the same error with a public repo?

@kcgthb

This comment has been minimized.

Show comment
Hide comment
@kcgthb

kcgthb May 16, 2017

I think there would be a use case here indeed, for public docs hosted out of a private repository.

One may still want to reference a private repo in the MkDocs configuration, to allow authorized users to edit the documentation through the "Edit on GitHub" link. But I'm not sure there's a way to retrieve stars/forks from a private repo, or even it would even make sense.

So maybe a sensible route would be to hide the Stars / Forks element if the information couldn't be fetched from the repo?

kcgthb commented May 16, 2017

I think there would be a use case here indeed, for public docs hosted out of a private repository.

One may still want to reference a private repo in the MkDocs configuration, to allow authorized users to edit the documentation through the "Edit on GitHub" link. But I'm not sure there's a way to retrieve stars/forks from a private repo, or even it would even make sense.

So maybe a sensible route would be to hide the Stars / Forks element if the information couldn't be fetched from the repo?

@alexcurtin

This comment has been minimized.

Show comment
Hide comment
@alexcurtin

alexcurtin May 16, 2017

@squidfunk Yes this works on a public repo. I believe its due to the fact that we store our documentation for mkdocs/material in a private repo. Can we add a check that when that call to github to find stars/forks fails (as its saying undefined) to just leave that part blank as @kcgthb suggests above?

@squidfunk Yes this works on a public repo. I believe its due to the fact that we store our documentation for mkdocs/material in a private repo. Can we add a check that when that call to github to find stars/forks fails (as its saying undefined) to just leave that part blank as @kcgthb suggests above?

@squidfunk

This comment has been minimized.

Show comment
Hide comment
@squidfunk

squidfunk May 16, 2017

Owner

Yes, that makes sense. Querying a private repository for stars you would have to expose credentials to your documentation site, which is pretty bad practice. Therefore it makes most sense to let the call fail silently and leaving the repository link as it is.

Owner

squidfunk commented May 16, 2017

Yes, that makes sense. Querying a private repository for stars you would have to expose credentials to your documentation site, which is pretty bad practice. Therefore it makes most sense to let the call fail silently and leaving the repository link as it is.

@squidfunk squidfunk self-assigned this May 16, 2017

@squidfunk squidfunk added the bug label May 16, 2017

@squidfunk

This comment has been minimized.

Show comment
Hide comment
@squidfunk

squidfunk May 16, 2017

Owner

Fixed in #331, will be released shortly as 1.6.3.

Owner

squidfunk commented May 16, 2017

Fixed in #331, will be released shortly as 1.6.3.

@squidfunk squidfunk closed this May 16, 2017

@alexcurtin

This comment has been minimized.

Show comment
Hide comment
@alexcurtin

alexcurtin May 16, 2017

@squidfunk Thanks dude! will this fix be installable via pip?

@squidfunk Thanks dude! will this fix be installable via pip?

@squidfunk

This comment has been minimized.

Show comment
Hide comment
@alexcurtin

This comment has been minimized.

Show comment
Hide comment
@alexcurtin

alexcurtin May 17, 2017

@squidfunk Thanks! You're amazing :)

@squidfunk Thanks! You're amazing :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment