-
Notifications
You must be signed in to change notification settings - Fork 27
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
Changes related to issue #79 deadlinks #80
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! :-)
poi.add(ValidationIssue.INVALID_URL); | ||
} | ||
} | ||
} | ||
|
||
private boolean validWebsiteURL(String urlString) { | ||
public boolean urlExists(String urlString) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add this:
if (urlString.contains("{{dead link")) {
return false;
}
Thanks! :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes made
Also, broken URL should be thrown away and not put into the output data. This can be a different pull request. |
Well anyways removing broken URL is been already handled previously i guess. I dint find it in any other place |
@Test | ||
public void testURLExistPositive(){ | ||
WebsiteURLValidator websiteURLValidator = new WebsiteURLValidator(); | ||
assertTrue(websiteURLValidator.urlExists("www.google.com")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should require a protocol (for instance https://) no?
url = new URL(urlString); | ||
|
||
// We want to check the current URL | ||
HttpURLConnection.setFollowRedirects(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation seems to be inconsistent.
No description provided.