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

Use a separate fake GWT module with the entry point. #81

Merged
merged 1 commit into from
Oct 26, 2018

Conversation

ringw
Copy link
Contributor

@ringw ringw commented Oct 25, 2018

This caused errors in some GWT applications, and GWTTest passes without it.

@coveralls
Copy link

coveralls commented Oct 25, 2018

Pull Request Test Coverage Report for Build 234

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 94.088%

Totals Coverage Status
Change from base Build 232: 0.0%
Covered Lines: 2960
Relevant Lines: 3146

💛 - Coveralls

@sjamesr
Copy link
Contributor

sjamesr commented Oct 25, 2018

This isn't right: now the GWT compiler does not descend into the Pattern.java source. The test will pass even if GWT-incompatible things are introduced into RE2J.

I think the best thing to do is to revert the part of your earlier change that moved RE2J.gwt.xml out of testdata and into sources.

@sjamesr
Copy link
Contributor

sjamesr commented Oct 25, 2018

Oh, but you need the <super-source> spec still... Well, maybe we have to have a separate RE2J-test.gwt.xml in testdata that includes RE2J.gwt.xml.

The fake entry point is necessary for GWTTest to build anything at all,
but causes errors in production GWT applications, which should use the
RE2J.gwt.xml under java/.
@ringw ringw changed the title Remove the fake GWT entry point. Use a separate fake GWT module with the entry point. Oct 26, 2018
@ringw
Copy link
Contributor Author

ringw commented Oct 26, 2018

Argh, I forgot that the entry point is necessary. I've moved RE2J-Fake with the entry point back to testdata/, and verified that the GWT test fails if I make any breaking changes to super/.../Characters.java.

@sjamesr
Copy link
Contributor

sjamesr commented Oct 26, 2018

This looks good.

Quick question: if people want to use RE2J from GWT, is it sufficient to put the gwt.xml file in the source jar that we publish to maven? Right now, only .java files end up in the -sources.jar archive.

@ringw
Copy link
Contributor Author

ringw commented Oct 26, 2018

Yeah, I think that's all that's needed. They should be able to include the source jar in the classpath to the GWT compiler, and add a dependency on "com.google.re2j.RE2J" in their own GWT module.

@sjamesr
Copy link
Contributor

sjamesr commented Oct 26, 2018

Excellent, in that case I'll take care of making sure the gwt.xml is included in the sources jar.

@sjamesr sjamesr merged commit 3bbd13b into google:master Oct 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants