Jekyll Community #5097

Merged
merged 3 commits into from Jul 13, 2016

Conversation

Projects
None yet
6 participants
@chrisfinazzo
Contributor

chrisfinazzo commented Jul 11, 2016

See #5057 for discussion related to this PR.

Mike Neumegen and others added some commits Jul 6, 2016

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Jul 11, 2016

Member

Why not just force push this to the existing PR (#5095)?

Member

pathawks commented Jul 11, 2016

Why not just force push this to the existing PR (#5095)?

@chrisfinazzo

This comment has been minimized.

Show comment
Hide comment
@chrisfinazzo

chrisfinazzo Jul 11, 2016

Contributor

Tl;dr Force pushing is gross, so I try to avoid it when possible. See @cauerego's comment in the link for a more nuanced explanation.

Contributor

chrisfinazzo commented Jul 11, 2016

Tl;dr Force pushing is gross, so I try to avoid it when possible. See @cauerego's comment in the link for a more nuanced explanation.

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Jul 11, 2016

Member

Force pushing is gross

One conversation that spans across several PRs is gross, but I don't mean to derail things here. Carry on.

Member

pathawks commented Jul 11, 2016

Force pushing is gross

One conversation that spans across several PRs is gross, but I don't mean to derail things here. Carry on.

@chrisfinazzo

This comment has been minimized.

Show comment
Hide comment
Contributor

chrisfinazzo commented Jul 11, 2016

@pathawks
hug it out

site/_data/docs.yml
@@ -40,6 +40,7 @@
- troubleshooting
- sites
- resources
+ - jekyllconf

This comment has been minimized.

@parkr

parkr Jul 12, 2016

Member

This needs to be removed 😄

@parkr

parkr Jul 12, 2016

Member

This needs to be removed 😄

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 12, 2016

Member

I have nothing against force pushes! The main repo is not really affected by force pushes.

Thanks for submitting this again, Chris. One comment above, then this LGTM.

Member

parkr commented Jul 12, 2016

I have nothing against force pushes! The main repo is not really affected by force pushes.

Thanks for submitting this again, Chris. One comment above, then this LGTM.

@chrisfinazzo

This comment has been minimized.

Show comment
Hide comment
@chrisfinazzo

chrisfinazzo Jul 12, 2016

Contributor

Ack! Missed that, will update.

Contributor

chrisfinazzo commented Jul 12, 2016

Ack! Missed that, will update.

@cauerego

This comment has been minimized.

Show comment
Hide comment
@cauerego

cauerego Jul 12, 2016

Going on the tangent for which I was mentioned here and without knowing if this is gross or not... Nothing against force pushes @parkr ?! Why not?

Going on the tangent for which I was mentioned here and without knowing if this is gross or not... Nothing against force pushes @parkr ?! Why not?

@chrisfinazzo

This comment has been minimized.

Show comment
Hide comment
@chrisfinazzo

chrisfinazzo Jul 12, 2016

Contributor

@cauerego, As you described, with great power comes great responsibility. For that way lies madness 😏

Contributor

chrisfinazzo commented Jul 12, 2016

@cauerego, As you described, with great power comes great responsibility. For that way lies madness 😏

@cauerego

This comment has been minimized.

Show comment
Hide comment
@cauerego

cauerego Jul 12, 2016

Yikes, now I'm completely lost @chrisfinazzo! I was also probably not clear: I'm just hoping Mr Moore could make me see a completely different angle from what I brought on the link you've pointed and in which I don't recall bringing up Mr Parker. 😄

Yikes, now I'm completely lost @chrisfinazzo! I was also probably not clear: I'm just hoping Mr Moore could make me see a completely different angle from what I brought on the link you've pointed and in which I don't recall bringing up Mr Parker. 😄

@chrisfinazzo

This comment has been minimized.

Show comment
Hide comment
@chrisfinazzo

chrisfinazzo Jul 12, 2016

Contributor

@cauerego As you indicated in the SO comment I linked, rewriting published history is strongly discouraged. It requires people to do scary things in order to incorporate new changes and is generally not a situation you want to find yourself in.

Even if it's only in a branch, force-pushing has that smell to it 👎🏻

Contributor

chrisfinazzo commented Jul 12, 2016

@cauerego As you indicated in the SO comment I linked, rewriting published history is strongly discouraged. It requires people to do scary things in order to incorporate new changes and is generally not a situation you want to find yourself in.

Even if it's only in a branch, force-pushing has that smell to it 👎🏻

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 13, 2016

Member

Did you check this on mobile? We might need to add a link to this page in the mobile nav.

Member

parkr commented Jul 13, 2016

Did you check this on mobile? We might need to add a link to this page in the mobile nav.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Jul 13, 2016

Contributor

@cauerego force pushes matter little on a development branch, that's what temporary development branches are for, forcing, cleaning and organizing a patch to send. In this case, it's just automatically done for you through Github.

Contributor

envygeeks commented Jul 13, 2016

@cauerego force pushes matter little on a development branch, that's what temporary development branches are for, forcing, cleaning and organizing a patch to send. In this case, it's just automatically done for you through Github.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 13, 2016

Member

Took a look locally. All is well. 👍

Member

parkr commented Jul 13, 2016

Took a look locally. All is well. 👍

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 13, 2016

Member

@jekyllbot: merge +site

Member

parkr commented Jul 13, 2016

@jekyllbot: merge +site

@jekyllbot jekyllbot merged commit b1674ed into jekyll:master Jul 13, 2016

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details

jekyllbot added a commit that referenced this pull request Jul 13, 2016

@chrisfinazzo

This comment has been minimized.

Show comment
Hide comment
@chrisfinazzo

chrisfinazzo Jul 13, 2016

Contributor

@parkr Yeah, looked at it with the iOS Simulator and everything seems to be in order. I am a bit wary of the padding in portrait orientation, but that's not a huge deal.
screen shot 2016-07-12 at 10 59 19 pm

Contributor

chrisfinazzo commented Jul 13, 2016

@parkr Yeah, looked at it with the iOS Simulator and everything seems to be in order. I am a bit wary of the padding in portrait orientation, but that's not a huge deal.
screen shot 2016-07-12 at 10 59 19 pm

@chrisfinazzo chrisfinazzo deleted the chrisfinazzo:jekyll-community branch Jul 13, 2016

stevecheckoway added a commit to stevecheckoway/jekyll that referenced this pull request Jul 24, 2016

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