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

Fix a link in deployment-methods.md #5244

Merged
merged 1 commit into from Aug 18, 2016

Conversation

Projects
None yet
6 participants
@mkasberg
Contributor

mkasberg commented Aug 14, 2016

The link to the scp deploy script was broken - it appears someone changed the link text and did not update the matching link text in the markdown footer. To prevent this problem from happening again, and for consistency with the rest of this markdown file, I changed both links to inline style links.

@mkasberg mkasberg referenced this pull request Aug 14, 2016

Closed

Link Broken in deployment-methods.md #5245

4 of 17 tasks complete
@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Aug 14, 2016

Contributor

👎 that script uses scss directly, it's preferred to do it via Jekyll itself since it's embedded.

Contributor

envygeeks commented Aug 14, 2016

👎 that script uses scss directly, it's preferred to do it via Jekyll itself since it's embedded.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Aug 14, 2016

Contributor

To note, I already know it was there, but now that it's been brought to attention, I question if it's still viable.

Contributor

envygeeks commented Aug 14, 2016

To note, I already know it was there, but now that it's been brought to attention, I question if it's still viable.

@mkasberg

This comment has been minimized.

Show comment
Hide comment
@mkasberg

mkasberg Aug 15, 2016

Contributor

That makes sense. I actually removed that line from my own version of the script. I was just trying to fix the broken link, since it looks bad on the website right now.

Is there a more preferable implementation of that script somewhere that we could link to?

Alternatively, since it's basically a one-liner, what if we just actually write the script into the markdown? Like this:

scp -r _site/* user@server:/home/user/public_html
Contributor

mkasberg commented Aug 15, 2016

That makes sense. I actually removed that line from my own version of the script. I was just trying to fix the broken link, since it looks bad on the website right now.

Is there a more preferable implementation of that script somewhere that we could link to?

Alternatively, since it's basically a one-liner, what if we just actually write the script into the markdown? Like this:

scp -r _site/* user@server:/home/user/public_html
@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Aug 15, 2016

Contributor

I am 👍 on just inlining it. That is what I thought when I seen it.

On Mon, Aug 15, 2016, 3:25 PM Mike Kasberg notifications@github.com wrote:

That makes sense. I actually removed that line from my own version of the
script. I was just trying to fix the broken link, since it looks bad on the
website right now.

Is there a more preferable implementation of that script somewhere that we
could link to?

Alternatively, since it's basically a one-liner, what if we just actually
write the script into the markdown? Like this:

scp -r _site/* user@server:/home/user/public_html


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#5244 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAGFsyOaYlMQzI655LAUGH-17GsD5NKaks5qgMtWgaJpZM4Jj9O8
.

Contributor

envygeeks commented Aug 15, 2016

I am 👍 on just inlining it. That is what I thought when I seen it.

On Mon, Aug 15, 2016, 3:25 PM Mike Kasberg notifications@github.com wrote:

That makes sense. I actually removed that line from my own version of the
script. I was just trying to fix the broken link, since it looks bad on the
website right now.

Is there a more preferable implementation of that script somewhere that we
could link to?

Alternatively, since it's basically a one-liner, what if we just actually
write the script into the markdown? Like this:

scp -r _site/* user@server:/home/user/public_html


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#5244 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAGFsyOaYlMQzI655LAUGH-17GsD5NKaks5qgMtWgaJpZM4Jj9O8
.

Fix a link in deployment-methods.md
The link to the scp deploy script was broken - it appears someone
changed the link text and did not update the matching link text in the
markdown footer.

Because the script is really simple, and the script originally linked
includes some unnecessary scss commands, let's just inline the script.
@mkasberg

This comment has been minimized.

Show comment
Hide comment
@mkasberg

mkasberg Aug 15, 2016

Contributor

Just updated the pull request to have it inlined.

Contributor

mkasberg commented Aug 15, 2016

Just updated the pull request to have it inlined.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Aug 16, 2016

Contributor

LGTM.

Contributor

envygeeks commented Aug 16, 2016

LGTM.

@envygeeks envygeeks added bug fix labels Aug 16, 2016

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Aug 17, 2016

Contributor

/cc @jekyll/documentation @jekyll/core

Contributor

envygeeks commented Aug 17, 2016

/cc @jekyll/documentation @jekyll/core

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Aug 17, 2016

Member

LGTM.

Member

DirtyF commented Aug 17, 2016

LGTM.

@mattr-

This comment has been minimized.

Show comment
Hide comment
@mattr-

mattr- Aug 18, 2016

Member

LGTM

@jekyllbot: merge +site

Member

mattr- commented Aug 18, 2016

LGTM

@jekyllbot: merge +site

@jekyllbot jekyllbot merged commit d0ae757 into jekyll:master Aug 18, 2016

1 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
jekyll/lgtm Approved by @envygeeks. Requires 1 more LGTM.
continuous-integration/travis-ci/pr The Travis CI build passed
Details

jekyllbot added a commit that referenced this pull request Aug 18, 2016

@albertvolkman

This comment has been minimized.

Show comment
Hide comment
@albertvolkman

albertvolkman Sep 15, 2016

Also, the text immediately following is an exact duplicate.

screenshot 2016-09-15 13 20 45

Also, the text immediately following is an exact duplicate.

screenshot 2016-09-15 13 20 45

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