Remove StaticRoutesGenerator#8049
Conversation
gmethvin
force-pushed
the
remove-static-routes
branch
from
3348ec7 to
c8803e9
Compare
richdougherty
left a comment
richdougherty
left a comment
There was a problem hiding this comment.
I suggested removing the sbt setting used to choose a different generator, since now there's only one kind. Not sure if that was something you were planning for a future PR or not.
There was a problem hiding this comment.
Can we get rid of the routesGenerator setting entirely now that there's only one type of generator?
There was a problem hiding this comment.
I was wondering that too, but thinking it doesn't hurt to leave it there just in case someone wants to configure a custom routes generator.
There was a problem hiding this comment.
That sounds fair enough. I guess I'm always a fan of reducing API surface if we can. :)
There was a problem hiding this comment.
well I think we should let it there. since sird is better and better, people can actually create a NoOpRoutesGenerator, which can be used with sird. This will save a lot of unnecessary IO while looking for route files. (and there are various other way's why people would create their own..)
There was a problem hiding this comment.
@schmitch wouldn't it be better to have Play provide a NoOpRoutesGenerator for people in that case?
There was a problem hiding this comment.
well I'm not sure how many people would actually need it and it's not really that hard to implement.
There was a problem hiding this comment.
Or they could just remove the RoutesCompiler plugin. We have a PlayService plugin in 2.7 that is more minimal and does not include the routes compiler.
I have no problem with leaving the option in, though. The main point of this PR is to avoid having to maintain the static routes compiler. I could also see value in allowing someone to provide a routes generator that uses a completely different mechanism to generate a router or customizes the default generator in some way. It's possible people are already doing that.
There was a problem hiding this comment.
I'm OK with leaving the option in. FYI I just changed my review to 👍 since I'm not requesting changes anymore.
4 participants
Removes the static routes generator, references the removal in the the migration guide, and removes references to the static routes generator in the documentation.
Fixes #8047.