Skip to content
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

Add option: IgnoreExternalBrokenLinks #140

Merged
merged 1 commit into from
Jan 25, 2020
Merged

Conversation

abhishalya
Copy link
Contributor

@abhishalya abhishalya commented Jan 4, 2020

Do let me know if it is useful, we particularly required this while checking links for julialang.org site which has a large number of links.

I'll add tests and docs soon, if this idea looks good enough.

Closes #139

cc @wjdp

@codecov
Copy link

codecov bot commented Jan 4, 2020

Codecov Report

Merging #140 into master will increase coverage by 0.53%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #140      +/-   ##
==========================================
+ Coverage   82.74%   83.27%   +0.53%     
==========================================
  Files          20       20              
  Lines        1153     1160       +7     
==========================================
+ Hits          954      966      +12     
+ Misses        179      174       -5     
  Partials       20       20
Impacted Files Coverage Δ
htmltest/check-link.go 97.75% <100%> (+0.02%) ⬆️
htmltest/check-generic.go 82.5% <100%> (+1.41%) ⬆️
htmltest/options.go 90.41% <100%> (+0.13%) ⬆️
htmltest/util.go 55.55% <0%> (+13.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57fe7e6...3ff9286. Read the comment docs.

@wjdp
Copy link
Owner

wjdp commented Jan 5, 2020

@abhishalya Saw your issue this morning. We've this exact problem on a site I help maintain, 👍 on the feature from me!

@wjdp
Copy link
Owner

wjdp commented Jan 5, 2020

I can see this is marked as WIP but thought I'd mention:

  • This will need tests
  • I'm unsure on the option name AllowExternalLinksToFail, we've no other options with the prefix "Allow". "Ignore" is quite prevalent but not sure if this feels right. If the other usages of "Ignore" imply demoting errors to warnings this fits, elsewise "Allow" may want adding for this meaning. I've a feeling the "Ignore" prefix has both "ignore completly" and "demote to warning" implementations, would be worth clearing these up in future.

@abhishalya
Copy link
Contributor Author

@wjdp I agree. How about IgnoreExternalBrokenLinks?

This adds a new option to produce a warning rather
than an error for broken external links. It is
quite useful for sites having hundreds of external
links.
@abhishalya abhishalya changed the title [WIP] Add AllowExternalLinksToFail Add option: IgnoreExternalBrokenLinks Jan 5, 2020
@abhishalya
Copy link
Contributor Author

@wjdp I've added the tests, can you have a look again :)

@abhishalya
Copy link
Contributor Author

@wjdp Any update on this one?

@wjdp
Copy link
Owner

wjdp commented Jan 25, 2020

Sorry about delay on this!

@wjdp wjdp merged commit 1171a50 into wjdp:master Jan 25, 2020
@abhishalya
Copy link
Contributor Author

No problem, thanks for the merge :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow failures for external links
2 participants