pidcodes / pidcodes.github.com

Website for pid.codes
http://pid.codes/
Other
486 stars 797 forks source link

Travis TODO #184

Open peternewman opened 8 years ago

peternewman commented 8 years ago
Arachnid commented 8 years ago

FYI, https://github.com/pidcodes/pidcodes.github.com/pull/186 is passing despite the owner field being incorrect.

peternewman commented 8 years ago

It did actually catch it, but as an external link, which we currently ignore, unless you manually review them.

Before: https://travis-ci.org/pidcodes/pidcodes.github.com/jobs/167023698#L1651

After: https://travis-ci.org/pidcodes/pidcodes.github.com/jobs/167102370#L1572

peternewman commented 7 years ago

Still not right, see the last two commits here: https://github.com/pidcodes/pidcodes.github.com/pull/195

peternewman commented 7 years ago

I've realised, it's not catching the broken org links, as the site is already coded to defend against them.

orgpath contains the actual, potentially incorrect URL https://github.com/pidcodes/pidcodes.github.com/blob/master/_layouts/pid.html#L2 But the link used on the page is org.url, which comes from https://github.com/pidcodes/pidcodes.github.com/blob/master/_layouts/pid.html#L3 and only matches if the two are the same already, otherwise you'll get nothing for org.url or org.title, but there will be nothing for the proofer to find either.

peternewman commented 7 years ago

Proved here: https://travis-ci.org/peternewman/pidcodes.github.com/jobs/179990067

Are you happy for me to make the change @Arachnid ? It would only make a difference if something gets merged with a broken link.

Arachnid commented 7 years ago

Good catch - please do!

peternewman commented 7 years ago

Done, see https://github.com/pidcodes/pidcodes.github.com/issues/198 . To be undone once we get some better suited method of checking the fields, like the hex markdown checks will require.

peternewman commented 7 years ago

Confirmed the link validation will catch people giving their org folder the wrong name, sorted in #198

peternewman commented 7 years ago

Some possible validation options: https://github.com/cityoffortworth/jekyll-data_validation https://github.com/18F/jekyll_frontmatter_tests

Arachnid commented 7 years ago

I'm reasonably happy with verifying the generated HTML; is there a big disadvantage to doing that, or stuff we can't catch?

peternewman commented 7 years ago

For starters, 000a and 000A are different folder names, so you could get two people grabbing the same PID for example (perhaps even force a specific case to fix the sorting).

I'm sure once it's in place, there will be other benefits that become easy wins, like ensuring people have filled in the required fields. Perhaps even validating for a license from the various types.

Arachnid commented 7 years ago

Yup, case sensitivity and accidental collisions because of that is something I've been worried about for a while. Good point that it's fixable with more sophisticated validation.

peternewman commented 7 years ago

Likewise https://travis-ci.org/pidcodes/pidcodes.github.com/jobs/195217329#L342 , we can validate people are using valid layouts.

Arachnid commented 7 years ago

I noticed the invalid layout when reviewing it. Throwing an error would be much more useful here.

peternewman commented 7 years ago

Agreed, I'm hoping https://github.com/jekyll/jekyll/pull/5832 will improve things when it gets merged.

tannewt commented 4 years ago

I've added a basic Python validator in #554. It only does 0x1xxx checking now but should be able to do case checking and file path validation easily. https://github.com/pidcodes/pidcodes.github.com/pull/554/files#diff-c595d105a360259a2077a56eae46bcf3

tannewt commented 3 years ago

The fourth bullet is done it #582