-
Notifications
You must be signed in to change notification settings - Fork 14
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
Consider moving expectJSONTypes into a plugin #90
Comments
I'm all for migrating specific functionality to plugins and out of core. 👍 I think that we should expect to see more async plugins that will need an async (Promise) plugin API. I think it might be advantageous to look into what would be required to support this before we push all the features out of core and then be forced to rewrite the plugin interface later. |
How would you like to proceed with this? Would you like me to break it out into a separate repo? Or to set up lerna? |
I'm game for trying lerna. If it doesn't work, we can just split it out to a separate repo. Do we want to make an IcedFrisby GitHub org so this isn't under my name going forward? I could make you an owner then. |
I've gone ahead and created the organization, and invited you and @cvega. That reminds me: @Fishbowler, I wonder if you would like to join forces with us. Would be great to have you on board as a maintainer! |
Do you want to move the repo over @MarkHerhold? |
I'd love to! My skills aren't super-strong, but I'm certainly eager. Got a few more ideas for plugins I want to write too, having read that I can extend existing functions. Also, the docs. Really wanna fix the docs. I'll do that first. |
On the note about expectJSONTypes, I make heavy use of it, but it always feels "different" from other checks. How could we manage upgrades on existing installs where it's in use? Is it enough to rely on people having solid package.json settings for packages? Maybe we deprecate and issue a warning akin to the multi globalSetup? |
@paulmelnikow can you send the invite again? I didn't get anything. 🤔 |
You should be able to access it here: https://github.com/icedfrisby |
Yea, I think a major semver bump is sufficient, with a mention in the release notes. |
@paulmelnikow xfered 👍 |
It occurs to me the original scope of this issue slots nicely with the recent removing of the Joi dependency, since Joi would likely be a dependency of this optional plugin. |
Just thinking about making a start on this. One of the migration paths might be to introduce a rename? |
I wanted to consider going one step further from #84 and moving
expectJSONTypes
into a plugin.I'd prefer to have an unopinionated core, and let people use whatever schema validation library they'd like. Supporting more schema validation libraries probably means more plugins, rather than adding other schema validation integrations into core.
I'd also like to strengthen our budding plugin ecosystem. Having good examples of plugins, and people using them, helps do that.
@MarkHerhold it seems like you're a big Joi user, and that there probably are many icedfrisby users who are using it, so continuing to have smooth integration is important. The plugin would be pretty smooth, working just as today, though requiring a couple lines of code to construct the mixin. That can be moved into a utility package so it doesn't need to be repeated in every file.
The advantage would be fewer tests in core, slightly less code, and a shining example of what a schema-validation plugin should look like.
We could use lerna to manage development of both modules in a single repo. Honestly our plugin infrastructure is pretty stable and so is the
expectJSONTypes
code, so there may not be any need for coordinated development. It might be less work to develop it in its own repo.The text was updated successfully, but these errors were encountered: