Skip to content

Conversation

@sant0s
Copy link
Contributor

@sant0s sant0s commented May 2, 2018

No description provided.

@asolntsev asolntsev self-assigned this Jun 4, 2018
@asolntsev asolntsev merged commit e06ed75 into playframework:master Jun 4, 2018
@asolntsev
Copy link
Contributor

@sant0s Thank you for the PR!

@sant0s
Copy link
Contributor Author

sant0s commented Jul 16, 2018

@asolntsev You're welcome. Any chance (Lighthouse) tickets 2170 to 2172 could be resolved?

@asolntsev
Copy link
Contributor

@sant0s Sure they can be resolved. Feel free to prepare a pull requests for them!

@sant0s sant0s deleted the lighthouse-2173-patch branch July 26, 2018 15:16
@sant0s sant0s restored the lighthouse-2173-patch branch July 26, 2018 15:16
@sant0s
Copy link
Contributor Author

sant0s commented Jul 26, 2018

@asolntsev That would be great as these issues refer to rather important functionality that was working in previous Play versions but has unfortunately been broken along the way. I had to comment application code as a workaround to make things work in the meantime. Perhaps other developers had to do the same.

Issues 2170 and 2171 refer to no longer being possible to access action context items from error templates. The corresponding pull requests are #1230 and #1231.

Issue 2172 refers to functional tests returning HTTP 200 instead of HTTP 404 for not found resources, thus violating REST compliance. I've created pull request #1234 but unfortunately it failed in Travis CI, so I've closed it. Do you think you - or someone from the Play team - could fix this issue?

@sant0s sant0s deleted the lighthouse-2173-patch branch July 26, 2018 17:21
@Fraserhardy
Copy link
Contributor

@sant0s @asolntsev I'm fairly certain issue 2172 is actually linked to issue 1849 which we've now fixed in this pr: #1246
It actually causes functional tests to return 200 for any exception! Which is critical in my opinion and has been open for quite some time.

@sant0s
Copy link
Contributor Author

sant0s commented Jul 27, 2018

@Fraserhardy @asolntsev I also find it critical, as it inhibits testing, therefore reducing coverage.

@asolntsev
Copy link
Contributor

@sant0s @Fraserhardy I added my comment to PRs #1230, #1231 and #1246. Thank you for your commitment!

@xael-fry xael-fry added this to the 1.5.1 milestone Aug 22, 2018
@sant0s
Copy link
Contributor Author

sant0s commented Apr 17, 2019

@sant0s @Fraserhardy I added my comment to PRs #1230, #1231 and #1246. Thank you for your commitment!

@asolntsev #1231 didn't go through although #1230 did (in Play! 1.5.2.). Could #1231 be included in the next Play! release?

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