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

[#1403] 🐛 Fix Groovy compilation unit that tries to write in applicat… #1406

Merged
merged 2 commits into from Apr 23, 2022

Conversation

holajsh
Copy link
Contributor

@holajsh holajsh commented Apr 21, 2022

Fix Groovy compilation unit that tries to write in application root folder

  • Restore the old reflection code that completely replace the OUTPUT phase of Groovy compilation unit
  • Fix the compilation exception handling that was producing cast error as not checking properly exception type

Fixes

Fixes #1403

…e in application root folder

* Restore the old reflection code that completely replace the OUTPUT phase of Groovy compilation unit
* Fix the compilation exception handling that was producing cast error as not checking properly exception type
@tazmaniax
Copy link
Collaborator

tazmaniax commented Apr 21, 2022

@holajsh here was the previous pull request #1368 to fix this that was some how lost. Note the comment that was included in the previous fix - probably worth bringing in that whole change as it was. It's bit disconcerting that this previous PR was lost.

@holajsh
Copy link
Contributor Author

holajsh commented Apr 21, 2022

@tazmaniax ok, I have added the comment again in my PR, thx. Hope indeed it won't be lost again ;)

@tomparle
Copy link
Contributor

@holajsh here was the previous pull request #1368 to fix this that was some how lost. Note the comment that was included in the previous fix - probably worth bringing in that whole change as it was. It's bit disconcerting that this previous PR was lost.

@tazmaniax my bad, although I tried my best I think I'm responsible for having messed this code when I rebased the Python 3 PR with master, because I got confused with what was new or not, since it was a revert to a previous code. Sorry and thank you for having detected and fixed this !

@tazmaniax
Copy link
Collaborator

@tomparle ah that's good to know where the regression might have come from, thanks for the heads up. So do you think this was the only regression or could there be others?

@tomparle
Copy link
Contributor

@tazmaniax to be honest, I tried to be careful with the rest of the code, but remembered the Groovy compilation part in particular because I was puzzled between new and old code. The rest should be ok.

@tazmaniax
Copy link
Collaborator

@tomparle ok great!

@xael-fry xael-fry merged commit ed2622d into playframework:master Apr 23, 2022
@xael-fry xael-fry added this to the 1.7.1 milestone Apr 23, 2022
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.

Temporary class of Groovy template is output directly under application dir
4 participants