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

Validator rollup #31316

Closed
wants to merge 4 commits into from
Closed

Validator rollup #31316

wants to merge 4 commits into from

Conversation

banaag
Copy link
Contributor

@banaag banaag commented Nov 24, 2020

@amp-owners-bot
Copy link

amp-owners-bot bot commented Nov 24, 2020

Hey @ampproject/wg-caching! These files were changed:

validator/js/engine/htmlparser-interface.js
validator/js/engine/validator.js
validator/js/engine/validator_test.js
validator/testdata/amp4ads_feature_tests/script_release_versions.html
validator/testdata/amp4ads_feature_tests/script_release_versions.out
validator/testdata/feature_tests/lts_extension_without_lts_runtime.html
validator/testdata/feature_tests/lts_extension_without_lts_runtime.out
validator/testdata/feature_tests/lts_runtime_after_extension.html
validator/testdata/feature_tests/lts_runtime_after_extension.out
validator/testdata/feature_tests/lts_runtime_and_extensions.html
validator/testdata/feature_tests/lts_runtime_and_extensions.out
validator/testdata/transformed_feature_tests/first_script_lts.html
+27 more

Copy link
Contributor

@honeybadgerdontcare honeybadgerdontcare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pushing this important pr forward

@honeybadgerdontcare
Copy link
Contributor

@rsimha we are doing a validation check against cdn.ampproject. in our validator code. Travis is complaining about it. Where do we submit an exception for this?

[19:21:16] Found forbidden: "cdn.ampproject." in validator/js/engine/htmlparser-interface.js:271:33

@honeybadgerdontcare
Copy link
Contributor

@rsimha I think I got it right in this commit.

Copy link
Contributor

@rcebulko rcebulko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approving build-system/tasks/presubmit-checks.js

@honeybadgerdontcare
Copy link
Contributor

There was a problem with this PR as it's missing some code that should have been included. That code is in PR #31373. To keep things simple we're closing this and once that PR is merged will do another rollup.

@banaag banaag deleted the validator-rollup branch January 5, 2021 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants