remove `Schema.analyzer()`
performantdata opened this issue · 11 comments
This can easily be replaced by the caller. Its presence creates a dependency from Schema to SchemaBasedDocumentAnalyzer.
Could you explain why this dependency is an issue?
Because I think that i can split off the schema representation classes, mostly Schema.scala, Context, Violation and some others. I think there should be modules that parallel ast and parser: one that creates the representation, and the representation itself.
To be sure I understand: you'd like to split sangria.schema and sangria.validation into their own sbt modules, right?
Personal opinion: I don't really see a big value in doing that. So if it's easy, why not. Otherwise, it'd be better to focus on extracting the auto derivation based on macros.
see #813
It does not tell me if it's easy, or if we will make so much changes that we will annoy all users.
Is #810 the only change needed to extract sangria.schema and sangria.validation into their own sbt modules?
Well I also don't know whether it's easy, but my intuition says it is. I'm not seeing a lot of dependencies other than what I've checked in. Oleg had a tendency to sprinkle helper methods around that save you one or two calls against methods from other (conceptual) modules.
But as long as this has tentative approval, I can rebase the branch that I'm using to try the separation off of these changes and see what else needs to be done. But I'd still want to know how I should proceed on #813.
Is #810 the only change needed to extract sangria.schema and sangria.validation into their own sbt modules?
I'm only focused on the former for now.
And keep in mind that we can always re-create these methods with proper separation of concerns using extension methods.
I'm ok reviewing a few PRs. And we should make sure not to break too many users.
But if the amount of work is more important, I'd prefer investing my time on the scala 3 migration.
I thought that these changes wouldn't be controversial. I might just close these and roll them all into one "schema separation" PR that motivates it all.
it'd be better to focus on extracting the auto derivation based on macros.
Turns out it depends on the schema classes. 😏