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

Default options for script/proof #5995

Merged
merged 2 commits into from Mar 31, 2017

Conversation

Projects
None yet
4 participants
@DirtyF
Member

DirtyF commented Mar 30, 2017

  • As http://jekyllrb.com/tutorials/navigation/ uses <a href="#"> links, allow-hash-href is now set to true by default
  • Set disable-external-url by default to avoid checking more than 2500 external URLs everytime
  • remove the lasting 404 errors
$ script/proof

Running ["ScriptCheck", "ImageCheck", "LinkCheck"] on ["./_site"] on *.html...
Ran on 122 files!
HTML-Proofer finished successfully.

/cc @jekyll/documentation

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Mar 30, 2017

Member

Could @jekyllbot periodically check for 404s?

Member

pathawks commented Mar 30, 2017

Could @jekyllbot periodically check for 404s?

Show outdated Hide outdated script/proof
@@ -32,4 +32,4 @@ bundle exec jekyll build -s $SOURCE -d $DESTINATION --trace
# 3.
msg "Proofing..."
time bundle exec htmlproofer ./$DESTINATION --url-ignore $INGORE_HREFS $@
time bundle exec htmlproofer ./$DESTINATION --allow-hash-href --disable-external --url-ignore $IGNORE_HREFS $@

This comment has been minimized.

@parkr

parkr Mar 31, 2017

Member

I'm not sure about --disable-external – I'd like to know when we're pointing to resources that are no longer there. What is failing? Can we periodically nix them?

@parkr

parkr Mar 31, 2017

Member

I'm not sure about --disable-external – I'd like to know when we're pointing to resources that are no longer there. What is failing? Can we periodically nix them?

This comment has been minimized.

@DirtyF

DirtyF Mar 31, 2017

Member

Right, let's deal with this in another PR, the primary goal was to fix the current errors.
It should be less painful to run the script when #5771 will be done anyway.

@DirtyF

DirtyF Mar 31, 2017

Member

Right, let's deal with this in another PR, the primary goal was to fix the current errors.
It should be less painful to run the script when #5771 will be done anyway.

This comment has been minimized.

@parkr

parkr Mar 31, 2017

Member

script/proof is not like our normal CI that needs to pass no matter what, so it's ok to leave it half broken. I'd say remove --disable-external from this PR.

@parkr

parkr Mar 31, 2017

Member

script/proof is not like our normal CI that needs to pass no matter what, so it's ok to leave it half broken. I'd say remove --disable-external from this PR.

This comment has been minimized.

@DirtyF

DirtyF Mar 31, 2017

Member

done

@DirtyF
change default proof options
- allow # links (used in tutorials)
- fix current 404 errors
@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Mar 31, 2017

Member

@pathawks you mean for example +docs would trigger script/proofbefore merging and @jekyllbot would post a comment in the PR if a 404 is present? That would be awesome indeed.

Member

DirtyF commented Mar 31, 2017

@pathawks you mean for example +docs would trigger script/proofbefore merging and @jekyllbot would post a comment in the PR if a 404 is present? That would be awesome indeed.

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Mar 31, 2017

Member

@DirtyF That would indeed be cool, but I was thinking more about just periodically (daily?) making sure everything we link to still exists. I imagine link rot is the problem we are trying to target with this, right?

It's important to make sure the external pages we link to still exist, but it's frustrating when that causes an unrelated PR to fail.

Member

pathawks commented Mar 31, 2017

@DirtyF That would indeed be cool, but I was thinking more about just periodically (daily?) making sure everything we link to still exists. I imagine link rot is the problem we are trying to target with this, right?

It's important to make sure the external pages we link to still exist, but it's frustrating when that causes an unrelated PR to fail.

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Mar 31, 2017

Member

@jekyllboy: merge +docs

Member

DirtyF commented Mar 31, 2017

@jekyllboy: merge +docs

@jekyllbot jekyllbot merged commit 0d02a25 into jekyll:master Mar 31, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

jekyllbot added a commit that referenced this pull request Mar 31, 2017

@DirtyF DirtyF deleted the DirtyF:script/proof branch Mar 31, 2017

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