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

Ignore the existing broken URLs #200

Merged
merged 83 commits into from
Jan 9, 2017
Merged

Conversation

peternewman
Copy link
Contributor

No description provided.

@peternewman
Copy link
Contributor Author

I should probably double check if any have just moved before we give up on them entirely.

@Arachnid
Copy link
Contributor

Arachnid commented Dec 3, 2016

Linkrot sets in fast! I predict a lot of updates to this file. :/

@peternewman peternewman mentioned this pull request Dec 3, 2016
8 tasks
@peternewman
Copy link
Contributor Author

Heh, well that's the other question, do we make it so this test has to pass for a merge or not? I'm inclined to say yes for a bit and see how it goes.

The fact we've only had 8 errors across all the PIDs so far bodes rather well I'd say.

@peternewman
Copy link
Contributor Author

@Misfittech you seem to have deleted your Gerbers by accident. Also would you mind checking the links I've updated to are the correct ones please?

@Arachnid
Copy link
Contributor

Is this ready for review & merge?

@peternewman
Copy link
Contributor Author

Not yet, I need to manually set all the other links to be proper links, then fix any new broken addresses. I think I'm going to add some new optional fields to PID and org, so expect those soon first.

@peternewman
Copy link
Contributor Author

Okay @Arachnid this is now green. For now this just fixes or blacklists any broken URLs, but you'll still need to manually look at Travis allowed failures to see if a new PR introduces broken links. Perhaps run it like that for a bit and see how many false positives it gives/how many existing sites disappear, then potentially look at integrating it as a required test?

@Arachnid Arachnid merged commit fe7b594 into pidcodes:master Jan 9, 2017
@Arachnid
Copy link
Contributor

Arachnid commented Jan 9, 2017

I assume I need to look at allowed failures and see if the count goes up? Eg, there's no easy way to see if the failures are new to the current PR?

@peternewman
Copy link
Contributor Author

The result should be green currently, as the count is zero. If an existing link breaks, and then a new PR is made, it will appear that the PR broke it, there's no trivial way to differentiate that. Although doing things like https://docs.travis-ci.com/user/cron-jobs/ and blacklisting broken links as soon as they appear could help.

I guess it should be possible to write something that runs during the PR, generates a list of all links, or all broken links from master at that point, and then uses that list to blacklist all existing links, then use that blacklist to run against the PR (so it only checks the PR links). But that's probably rather complex for the sake of a few web URLs...

As I say, the balance is between removing it from allowed failures, so you don't have to check, and the Travis merge will complain if there are broken links, balanced against dealing with random links dropping off as false positives.

@Arachnid
Copy link
Contributor

Right, gotcha. Thanks again for all the travis help!

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.

2 participants