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

🐛 Catch up java validator to Doc Css logic #28313

Merged
merged 24 commits into from May 19, 2020

Conversation

GeorgeLuo
Copy link
Contributor

  • Length check of css moved to Doc Css, clean Css length spec references

@amp-owners-bot
Copy link

amp-owners-bot bot commented May 11, 2020

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

validator/java/BUILD
validator/java/WORKSPACE
validator/java/build.sh
validator/java/exclude-pmd.properties
validator/java/pom.xml
validator/java/src/main/java/dev/amp/validator/AMPHtmlHandler.java
validator/java/src/main/java/dev/amp/validator/CdataMatcher.java
validator/java/src/main/java/dev/amp/validator/Context.java
validator/java/src/main/java/dev/amp/validator/ExtensionsContext.java
validator/java/src/main/java/dev/amp/validator/ParsedValidatorRules.java
validator/java/src/main/java/dev/amp/validator/ReferencePointMatcher.java
validator/java/src/main/java/dev/amp/validator/TagStack.java
validator/java/src/main/java/dev/amp/validator/ValidateTagResult.java
validator/java/src/main/java/dev/amp/validator/css/CssParsingConfig.java
validator/java/src/main/java/dev/amp/validator/css/Declaration.java
validator/java/src/main/java/dev/amp/validator/css/ParsedDocCssSpec.java
validator/java/src/main/java/dev/amp/validator/css/QualifiedRule.java
validator/java/src/main/java/dev/amp/validator/parser/AMPHtmlParser.java
validator/java/src/main/java/dev/amp/validator/utils/AttributeSpecUtils.java
validator/java/src/main/java/dev/amp/validator/utils/CssSpecUtils.java
validator/java/src/main/java/dev/amp/validator/utils/ExtensionsUtils.java
validator/java/src/main/java/dev/amp/validator/utils/TagSpecUtils.java
validator/java/src/main/java/dev/amp/validator/visitor/ImportantPropertyVisitor.java
validator/java/src/main/java/dev/amp/validator/visitor/InvalidDeclVisitor.java
validator/java/src/main/java/dev/amp/validator/visitor/UrlFunctionVisitor.java
validator/java/src/main/proto/validator.proto
validator/java/src/test/java/dev/amp/validator/CdataMatcherTest.java
validator/java/src/test/java/dev/amp/validator/ContextTest.java
validator/java/src/test/java/dev/amp/validator/CssLengthTest.java
validator/java/src/test/java/dev/amp/validator/ParsedValidatorRulesTest.java
validator/java/src/test/java/dev/amp/validator/TagSpecDispatchTest.java
validator/java/src/test/java/dev/amp/validator/TagStackTest.java
validator/java/src/test/java/dev/amp/validator/utils/AttributeSpecUtilsTest.java

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@nhant01
Copy link
Contributor

nhant01 commented May 11, 2020

@googlebot I consent

Copy link
Member

@Gregable Gregable left a comment

Choose a reason for hiding this comment

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

This is not a thorough review for correctness, but Approved to merge.

@Gregable
Copy link
Member

Let me know if you would like me to make the merge, I don't know if you have merge rights or not.

@GeorgeLuo
Copy link
Contributor Author

don't have merge rights, please merge

@Gregable Gregable added cla: yes and removed cla: no labels May 19, 2020
@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@Gregable
Copy link
Member

I think Nhan's consent isn't getting picked up from the bot because he quoted it, ie: It starts with a > symbol. Trying to force the bot to ignore.

@Gregable Gregable added cla: yes and removed cla: no labels May 19, 2020
@rsimha
Copy link
Contributor

rsimha commented May 19, 2020

Drive-by comment: We had to disable the java validator tests due to #28497. You'll need to re-run the checks for this PR after rebasing on master and pulling in #28498.

@Gregable
Copy link
Member

It looks like we're hitting the keyserver issue reported in another thread now.

@GeorgeLuo
Copy link
Contributor Author

that lessens priority to do as rsimha indicates, correct? rather than raises it?

@rsimha
Copy link
Contributor

rsimha commented May 19, 2020

I'll let Greg comment on the priority of this PR :)

Meanwhile, if you want to be able to merge it, just push a new commit (or rebase), and it should be able to run the other Travis checks while bypassing the keyserver problem.

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@nhant01
Copy link
Contributor

nhant01 commented May 19, 2020

@googlebot I consent

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@Gregable Gregable merged commit b84f341 into ampproject:master May 19, 2020
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.

None yet

6 participants