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

Fixed GWT DTD URL #100

Merged
merged 1 commit into from
May 23, 2017
Merged

Fixed GWT DTD URL #100

merged 1 commit into from
May 23, 2017

Conversation

Darsstar
Copy link
Contributor

@Darsstar Darsstar commented May 22, 2017

  1. Eclipse warning are annoying.
  2. This is an open source project
  3. ???
  4. profit!

This change is Reviewable

1. Eclipse warning are annoying.
2. This is an open source project
3. ???
4. profit!
@CLAassistant
Copy link

CLAassistant commented May 22, 2017

CLA assistant check
All committers have signed the CLA.

@hesara
Copy link
Contributor

hesara commented May 23, 2017

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.


src/main/resources/AppWidgetset.tmpl, line 3 at r1 (raw file):

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE module PUBLIC "-//Google Inc.//DTD Google Web Toolkit ${gwt.version}//EN"
        "http://gwtproject.org/doctype/${gwt.version}/gwt-module.dtd">

To me, this looks logically correct assuming gwt.version is up to date, but it seems to me that the GWT project hasn't published a DTD for GWT 2.8.1 - the 2.8.0 DTD would work, but not with the variable. We should definitely use GWT version 2.8.1, and perhaps the GWT project should also be prodded about this.


Comments from Reviewable

@Darsstar
Copy link
Contributor Author

Review status: 0 of 1 files reviewed at latest revision, all discussions resolved.


src/main/resources/AppWidgetset.tmpl, line 3 at r1 (raw file):

Previously, hesara (Henri Sara) wrote…

To me, this looks logically correct assuming gwt.version is up to date, but it seems to me that the GWT project hasn't published a DTD for GWT 2.8.1 - the 2.8.0 DTD would work, but not with the variable. We should definitely use GWT version 2.8.1, and perhaps the GWT project should also be prodded about this.

Consider the GWT project prodded: gwtproject/gwt-site#240


Comments from Reviewable

@hesara
Copy link
Contributor

hesara commented May 23, 2017

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@hesara hesara merged commit 66e0fdc into vaadin:master May 23, 2017
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

Successfully merging this pull request may close these issues.

None yet

3 participants