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

Question about Review.Rule.newModuleRuleSchemaUsingContextCreator #166

Open
lue-bird opened this issue Sep 24, 2023 · 2 comments
Open

Question about Review.Rule.newModuleRuleSchemaUsingContextCreator #166

lue-bird opened this issue Sep 24, 2023 · 2 comments

Comments

@lue-bird
Copy link
Contributor

lue-bird commented Sep 24, 2023

Review.Rule.newModuleRuleSchema allows adding for example a withElmJsonModuleVisitor.
However, newModuleRuleSchemaUsingContextCreator cannot be used with that visitor because the resulting schema doesn't have the phantom field canCollectProjectData : ().

As far as I can see, it is not possible to access the Elm.Project.Project/ElmJsonKey using a ContextCreator or any other method.

First, is this intentional somehow? Converting it to a project rule seems easy enough (and the result is probably nicer anyway).
And if not, should we add the phantom field in v3? Or would it be nicer to introduce something like withElmJson etc for the ContextCreator?

Edit: I can see that the current implementation of fromModuleRuleSchema would not allow the first suggested change easily. I'll try to poke a bit to see if this is necessary.

@lue-bird lue-bird changed the title Question about Review.Rule.newModuleRuleSchemaUsingContextCreator and Question about Review.Rule.newModuleRuleSchemaUsingContextCreator Sep 24, 2023
@jfmengels
Copy link
Owner

jfmengels commented Sep 24, 2023

This is intentional yes, people should write a project rule instead. I usually remember the reason, but for some reason I can't put my finger on it now 😥

@lue-bird
Copy link
Contributor Author

lue-bird commented Sep 24, 2023

If you do remember the reason some future time by chance, it would be nice to add the reason to the documentation of newModuleRuleUsingContextCreator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants