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

Travis TODO #184

Open
4 of 8 tasks
peternewman opened this issue Oct 7, 2016 · 17 comments
Open
4 of 8 tasks

Travis TODO #184

peternewman opened this issue Oct 7, 2016 · 17 comments

Comments

@peternewman
Copy link
Contributor

peternewman commented Oct 7, 2016

@Arachnid
Copy link
Contributor

FYI, #186 is passing despite the owner field being incorrect.

@peternewman
Copy link
Contributor Author

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
Copy link
Contributor Author

Still not right, see the last two commits here:
#195

@peternewman
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

Good catch - please do!

@peternewman
Copy link
Contributor Author

Done, see #198 . To be undone once we get some better suited method of checking the fields, like the hex markdown checks will require.

@peternewman
Copy link
Contributor Author

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

@peternewman
Copy link
Contributor Author

@Arachnid
Copy link
Contributor

Arachnid commented Dec 2, 2016

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

@peternewman
Copy link
Contributor Author

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
Copy link
Contributor

Arachnid commented Dec 2, 2016

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
Copy link
Contributor Author

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

@Arachnid
Copy link
Contributor

Arachnid commented Feb 7, 2017

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

@peternewman
Copy link
Contributor Author

Agreed, I'm hoping jekyll/jekyll#5832 will improve things when it gets merged.

@tannewt
Copy link
Collaborator

tannewt commented Sep 16, 2020

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
Copy link
Collaborator

tannewt commented Nov 18, 2020

The fourth bullet is done it #582

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

No branches or pull requests

3 participants