Skip to content
/ mkdocs Public
  • Sponsor
  • Notifications You must be signed in to change notification settings
  • Fork 2.5k
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

Check for CNAME file when using gh-deploy #285

Merged
merged 1 commit into from
Dec 16, 2014

Conversation

major
Copy link
Contributor

@major major commented Dec 15, 2014

If a CNAME file exists in the gh-pages branch, we should read it and use that URL as the expected GitHub pages location. For branches without a CNAME file, we will try to determine the URL using the origin URL.

# This GitHub pages repository has a CNAME configured.
with(open('CNAME', 'r')) as f:
cname_host = f.read()
print('Your documentation should be available shortly at: http://' + cname_host)
Copy link
Member

Choose a reason for hiding this comment

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

Would it make more sense to return here rather than indenting everything below?

@major
Copy link
Contributor Author

major commented Dec 15, 2014

Good point. I'll go adjust.

@major major force-pushed the enable-ghdeploy-cname branch from 2876a97 to 31427fe Compare December 15, 2014 20:43
@major
Copy link
Contributor Author

major commented Dec 15, 2014

@d0ugal That simplified it a bit. ;) Let me know how that looks.

@tomchristie
Copy link
Member

The messaging should also keep in mind that the CNAME file doesn't garuntee it'll be at that hostname. Might we want a 'Assuming your DNS records are setup correctly' prefix?

@major major force-pushed the enable-ghdeploy-cname branch from 31427fe to cd3d680 Compare December 15, 2014 21:05
@major
Copy link
Contributor Author

major commented Dec 15, 2014

Another good point. One would assume they would already be configured but... :trollface:

@@ -14,7 +14,18 @@ def gh_deploy(config):
except:
return

# TODO: Also check for CNAME file
# Does this repository have a CNAME set for GitHub pages?
cname_host = None
Copy link
Member

Choose a reason for hiding this comment

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

This line is redundant.

@d0ugal
Copy link
Member

d0ugal commented Dec 16, 2014

One minor nit, otherwise LGTM.

If a CNAME file exists in the gh-pages branch, we should read it and use that URL as the expected GitHub pages location.  For branches without a CNAME file, we will try to determine the URL using the origin URL.
@major major force-pushed the enable-ghdeploy-cname branch from cd3d680 to 6b558dd Compare December 16, 2014 13:50
@major
Copy link
Contributor Author

major commented Dec 16, 2014

Awesome. Just fixed that.

d0ugal added a commit that referenced this pull request Dec 16, 2014
Check for CNAME file when using gh-deploy
@d0ugal d0ugal merged commit b8e4cab into mkdocs:master Dec 16, 2014
@d0ugal
Copy link
Member

d0ugal commented Dec 16, 2014

Thanks!

@d0ugal d0ugal added this to the 0.12.0 milestone Dec 16, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants