Skip to content

First step in decoupling GT from framework#227

Draft
cies wants to merge 5 commits intomainfrom
groovyless-framework
Draft

First step in decoupling GT from framework#227
cies wants to merge 5 commits intomainfrom
groovyless-framework

Conversation

@cies
Copy link
Member

@cies cies commented Aug 13, 2023

Fixes: #65

@cies
Copy link
Member Author

cies commented Aug 13, 2023

I see some tests are failing, while all seemed to work running ./gradlew check from the project root locally. Will look into it soon.

@cies
Copy link
Member Author

cies commented Aug 13, 2023

Fixed some, but they still go wrong if I run all tests at once. Maybe tests cannot run two instances of Play.

xabolcs
xabolcs reviewed Aug 18, 2023
View reviewed changes
cies
cies commented Dec 5, 2023
View reviewed changes
fastergt/src/play/template2/JavaExtensions.java
return bytes / 1048576L + "MB";
}
return bytes / 1073741824L + "GB";
return Utils.formatSize(bytes);
Copy link
Member Author

@cies cies Dec 5, 2023

Choose a reason for hiding this comment

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

This is used in framework so it cannot be moved out. Hence I've put it in Utils.

framework/src/play/templates/Template.java
* @return The template result as string
*/
protected abstract String internalRender(Map<String, Object> args);
public abstract String internalRender(Map<String, Object> args);
Copy link
Member Author

@cies cies Dec 5, 2023

Choose a reason for hiding this comment

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

Needs to be accessible to the fastergt library/package/code.

replay-tests/helloworld/test/ui/hello/server-error-template.html
<!DOCTYPE html>

<html>
<!doctype html>
Copy link
Member Author

@cies cies Dec 5, 2023

Choose a reason for hiding this comment

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

modernize

framework/src/play/mvc/results/Error.java
private static final Logger logger = LoggerFactory.getLogger(Error.class);

private final int status;
protected final int status;
Copy link
Member Author

@cies cies Dec 5, 2023

Choose a reason for hiding this comment

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

Needed for making NotFound and Unauthorized subclasses of Error.

framework/src/errors/404.html
@@ -1,18 +1,16 @@
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
<!doctype html>
Copy link
Member Author

@cies cies Dec 5, 2023

Choose a reason for hiding this comment

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

modernize

@asolntsev asolntsev added this to the 2.4.0 milestone Dec 16, 2023
@asolntsev
Copy link
Contributor

asolntsev commented Dec 24, 2023 •
edited
Loading

@cies Finally, I started reviewing this PR.
To be honest, it's hard to me to review such a big change. For example, I didn't yet understand why it was needed to move play.template.ExecutableTemplate to package play.template2 etc.

My initial idea was just to move all groovy-related code from framework to a separate module gt (like "groovy templates", similar to "fastergt"). It seemed to me a very simple and minimal change.

Do you want me to try it out? Or you have some own plan?

@asolntsev
Copy link
Contributor

asolntsev commented Dec 30, 2023

@cies Look, this is my try: #316

@xabolcs xabolcs mentioned this pull request Jan 2, 2024
Merged
@cies
Copy link
Member Author

cies commented Jan 2, 2024 •
edited
Loading

@asolntsev thanks for looking into it!

I didn't yet understand why it was needed to move play.template.ExecutableTemplate to package play.template2 etc.

It's been a while, but looking at the code I think I moved it because this class depends on groovy, and the whole idea was to make framework not depend on that. I could have left it in the same package, I guess I just wanted the play.template package to be agnostic to the templating solution, or I simple did not get the meaning of the template2 package in relation to the template package.

Not moving play.template.ExecutableTemplate, FastTags, TagContext and GroovyInlineTags is --obviously-- completely okay with me. In that case we may also be able to keep Template.internalRender(...) protected (in stead of making it public).

My initial idea was just to move all groovy-related code from framework to a separate module gt (like "groovy templates", similar to "fastergt"). It seemed to me a very simple and minimal change.

That's my idea too! I was aiming a little higher: I wanted all GT-code (actual templates) to be eventually moved out of framework of which my PR was just step 1/2.

Please see the discussion of the issue.

@asolntsev asolntsev modified the milestones: 2.4.0, 2.5.0 Mar 22, 2024
@cies cies removed this from the 2.5.0 milestone May 22, 2024
@cies cies changed the title First step in decoupling GT from framework First step in decoupling GT from framework [dont merge] May 22, 2024
@cies cies changed the title First step in decoupling GT from framework [dont merge] First step in decoupling GT from framework May 23, 2024
@cies cies marked this pull request as draft May 23, 2024 12:47
@xabolcs xabolcs assigned xabolcs and unassigned cies Sep 2, 2025
@xabolcs
Copy link
Collaborator

xabolcs commented Sep 2, 2025 •
edited
Loading

I'll continue this as:

My aim to allow simple (e.g. backends) apps to skip fastergt dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xabolcs xabolcs xabolcs left review comments

Assignees

@xabolcs xabolcs

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove Groovy dependencies from framework

3 participants

@cies @asolntsev @xabolcs