Update continuous-integration.md #3891

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
4 participants
@johanbove
Contributor

johanbove commented Aug 5, 2015

More information added after having some trouble getting Travis to execute with the existing explanation.

Update continuous-integration.md
More information added after having some trouble getting Travis to execute with the existing explanation.
@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Aug 5, 2015

I think this part can be modified to simply mention wdm because there nothing of significant difference here, even better would be just to find a way to context into the Windows docs because there is really nothing here that isn't mentioned there and differs from Linux and Mac.

I think this part can be modified to simply mention wdm because there nothing of significant difference here, even better would be just to find a way to context into the Windows docs because there is really nothing here that isn't mentioned there and differs from Linux and Mac.

This comment has been minimized.

Show comment
Hide comment
@johanbove

johanbove Aug 5, 2015

Owner

Thanks for the feedback. I actually added this example since I was not entirely sure where to place the gem jekyll and gem html-proofer lines. But now that I see this, it's pretty obvious actually. I'm still a Ruby newbie.

Owner

johanbove replied Aug 5, 2015

Thanks for the feedback. I actually added this example since I was not entirely sure where to place the gem jekyll and gem html-proofer lines. But now that I see this, it's pretty obvious actually. I'm still a Ruby newbie.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Aug 5, 2015

For some it's "executable bit" but for most it's probably "exectuable attribute" so I would phrase that "needs to have the exectuable attribute set" rather than "exectuable rights."

For some it's "executable bit" but for most it's probably "exectuable attribute" so I would phrase that "needs to have the exectuable attribute set" rather than "exectuable rights."

This comment has been minimized.

Show comment
Hide comment
@johanbove

johanbove Aug 5, 2015

Owner

Thanks for the wording pointer. Will change this.

Owner

johanbove replied Aug 5, 2015

Thanks for the wording pointer. Will change this.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Aug 5, 2015

The bit about "chmod" is highly irrelevent without additional information IMO. I can already infer as much as you just forced me to read, perhaps link to the GNU coreutils page for chmod. http://www.gnu.org/software/coreutils/manual/html_node/chmod-invocation.html#chmod-invocation

The bit about "chmod" is highly irrelevent without additional information IMO. I can already infer as much as you just forced me to read, perhaps link to the GNU coreutils page for chmod. http://www.gnu.org/software/coreutils/manual/html_node/chmod-invocation.html#chmod-invocation

This comment has been minimized.

Show comment
Hide comment
@johanbove

johanbove Aug 5, 2015

Owner

Ok, gotcha. Will remove the line altogether without a link to chmod since this would take the reader too far away from the topic of the doc.

Owner

johanbove replied Aug 5, 2015

Ok, gotcha. Will remove the line altogether without a link to chmod since this would take the reader too far away from the topic of the doc.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Aug 5, 2015

This is a workaround, not a solution. The solution is "bundle update" on your local system (which isn't a valid solution on Windows) and check-in the new Gemfile.lock or don't check-in the Gemfile.lock at all. The workaround is to simply delete the Gemfile.lock as you suggested. I think we should encourage people not to check-in the Gemfile.lock and learn to properly use dependency locking inside of their Gemfile but that's for another route, can we just suggest they don't check-in Gemfile.lock at all?

This is a workaround, not a solution. The solution is "bundle update" on your local system (which isn't a valid solution on Windows) and check-in the new Gemfile.lock or don't check-in the Gemfile.lock at all. The workaround is to simply delete the Gemfile.lock as you suggested. I think we should encourage people not to check-in the Gemfile.lock and learn to properly use dependency locking inside of their Gemfile but that's for another route, can we just suggest they don't check-in Gemfile.lock at all?

This comment has been minimized.

Show comment
Hide comment
@johanbove

johanbove Aug 5, 2015

Owner

Ok, I willl rephrase and add in the advice not to check in the Gemfile.lock file.
Should we add how to add an entry to .gitignore, or will this take us too far?

Owner

johanbove replied Aug 5, 2015

Ok, I willl rephrase and add in the advice not to check in the Gemfile.lock file.
Should we add how to add an entry to .gitignore, or will this take us too far?

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Aug 5, 2015

Personally I think that's a story for another day much like the dependency locking.

Personally I think that's a story for another day much like the dependency locking.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Aug 5, 2015

Contributor

Love it @johanbove great work. Just a few minor suggestions.

Contributor

envygeeks commented Aug 5, 2015

Love it @johanbove great work. Just a few minor suggestions.

@johanbove

This comment has been minimized.

Show comment
Hide comment
@johanbove

johanbove Aug 5, 2015

Contributor

You're welcome. I will go over your feedback and update my version during the course of the day.

Contributor

johanbove commented Aug 5, 2015

You're welcome. I will go over your feedback and update my version during the course of the day.

Update continuous-integration.md
Changed according to feedback by @envygeeks
@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Aug 5, 2015

Member

Merged in d3c327e

Member

parkr commented Aug 5, 2015

Merged in d3c327e

@parkr parkr closed this Aug 5, 2015

parkr added a commit that referenced this pull request Aug 5, 2015

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

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