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

Meta: use the common deploy.sh #121

Merged
merged 2 commits into from
Oct 31, 2017
Merged

Meta: use the common deploy.sh #121

merged 2 commits into from
Oct 31, 2017

Conversation

foolip
Copy link
Member

@foolip foolip commented Sep 3, 2017

No description provided.

@foolip
Copy link
Member Author

foolip commented Sep 3, 2017

This successfully deployed a branch snapshot to the new server. I'm not sure whether the checker step is equivalent before and after since the method used is different. @sideshowbarker?

@annevk
Copy link
Member

annevk commented Sep 4, 2017

Would the output from the checker even show if it's non-fatal with the current setup? It seems like it might not be shown, which isn't ideal.

@annevk
Copy link
Member

annevk commented Sep 4, 2017

https://travis-ci.org/whatwg/encoding/builds/232362067 is from before @sideshowbarker added the filter. Shows lots of warnings "Text run is not in Unicode Normalization Form C." that are typically useful to know about, but we know are not a problem for this document, which is why we filter them.

@annevk
Copy link
Member

annevk commented Sep 4, 2017

(I'd actually argue for making such warnings an error for WHATWG documents, except when we decide it's not an error.)

@foolip
Copy link
Member Author

foolip commented Sep 4, 2017

https://travis-ci.org/whatwg/encoding/builds/271487608 is the run with these changes, and the checker must have run and passed, even though there's no output. So I guess the common deploy.sh is more lenient than the previous setup? But https://checker.html5.org/?doc=https%3A%2F%2Fencoding.spec.whatwg.org%2F shows no errors, is that expected?

@annevk
Copy link
Member

annevk commented Sep 4, 2017

Oh, then the problem is that you don't run the checker on all the other HTML files that end up being generated.

@foolip
Copy link
Member Author

foolip commented Sep 4, 2017

@sideshowbarker, the common deploy.sh does /usr/lib/jvm/java-8-oracle/jre/bin/java -jar vnu.jar --skip-non-html "$WEB_ROOT", doesn't that traverse and check all HTML files?

@sideshowbarker
Copy link
Contributor

the common deploy.sh does /usr/lib/jvm/java-8-oracle/jre/bin/java -jar vnu.jar --skip-non-html "$WEB_ROOT", doesn't that traverse and check all HTML files?

Yes, that traverses the entire $WEB_ROOT tree and runs the checker on every HTML file it finds

@sideshowbarker
Copy link
Contributor

I'm not sure whether the checker step is equivalent before and after since the method used is different.

I’m not sure either. Is the difference that in case it’s sending the documents to https://checker.html5.org/ for checking using checker’s network API, and in the other case it’s downloading the jar and checking the documents locally?

If so, I think we want to be using the checker’s network API—not the jar—for all specs except the HTML spec. See whatwg/meta#4 (comment) for why.

@annevk
Copy link
Member

annevk commented Sep 5, 2017

Well, Encoding also has a lot of HTML files and I suspect they may end up being rather big in total, but I haven't verified.

The main problem here is that @foolip removed filtering for warnings, but the new output doesn't show warnings. So where did those go?

@foolip
Copy link
Member Author

foolip commented Sep 5, 2017

OK, so I've run the existing script locally a bit to understand what these warnings are. https://encoding.spec.whatwg.org/big5.html has warnings like "Text run is not in Unicode Normalization Form C.", which show up when using the https://checker.html5.org/ API. The warnings also show up when using java -jar vnu.jar encoding.spec.whatwg.org/big5.html locally, but the exit code is 0.

@sideshowbarker, is there a way of making errors into warnings and the suppressing some warnings using the vnu.jar?

@foolip
Copy link
Member Author

foolip commented Sep 5, 2017

Hmm, looks like --filterfile and --filterpattern are the way to suppress, but how to make warnings errors?

@sideshowbarker
Copy link
Contributor

There previously was no way of making the checker exit non-zero if there are only warnings, but I just today added a new --Werror option, so it’s in the current jar:

https://sideshowbarker.net/nightlies/jar/vnu.jar

@foolip
Copy link
Member Author

foolip commented Sep 9, 2017

Yay, thanks! I think we should add a CHECKER_FILTER environment variable that is passed to --filterpattern, and then this will be unblocked. Although, I don't think I'll try to retain the logic that's clever about which files to validate, so deploys will start taking longer. @annevk, is that acceptable?

@foolip foolip force-pushed the use-common-deploy branch from b00cbc4 to 5f02fcf Compare October 30, 2017 14:13
@foolip
Copy link
Member Author

foolip commented Oct 30, 2017

This is ready for review again, best reviewed together with whatwg/whatwg.org#111.

foolip added a commit to whatwg/whatwg.org that referenced this pull request Oct 31, 2017
@foolip foolip merged commit f381389 into master Oct 31, 2017
@foolip foolip deleted the use-common-deploy branch October 31, 2017 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants